Bug 189286 - [LayoutTests] Add digest access authentication test.
Summary: [LayoutTests] Add digest access authentication test.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Basuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-09-04 17:00 PDT by Basuke Suzuki
Modified: 2018-09-07 21:04 PDT (History)
8 users (show)

See Also:


Attachments
PATCH (4.59 KB, patch)
2018-09-04 17:02 PDT, Basuke Suzuki
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (3.21 MB, application/zip)
2018-09-04 19:40 PDT, EWS Watchlist
no flags Details
PATCH (4.52 KB, patch)
2018-09-05 10:33 PDT, Basuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Basuke Suzuki 2018-09-04 17:00:38 PDT
There seems no simple Digest authentication test. Add it.
Comment 1 Basuke Suzuki 2018-09-04 17:02:55 PDT
Created attachment 348875 [details]
PATCH
Comment 2 EWS Watchlist 2018-09-04 19:40:26 PDT
Comment on attachment 348875 [details]
PATCH

Attachment 348875 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9096805

New failing tests:
css3/filters/backdrop/add-remove-add-backdrop-filter.html
Comment 3 EWS Watchlist 2018-09-04 19:40:28 PDT
Created attachment 348886 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 4 Alexey Proskuryakov 2018-09-05 09:49:12 PDT
Comment on attachment 348875 [details]
PATCH

View in context: https://bugs.webkit.org/attachment.cgi?id=348875&action=review

> LayoutTests/http/tests/resources/digest-auth/digest.php:1
> +<?php

Calculating a digest response in PHP is quite cunning! However, I can't help but wonder if this would be easier to set up with an Apache config.

> LayoutTests/http/tests/xmlhttprequest/digest-auth-expected.txt:2
> +PASS
> +User: everybody

This is quite confusing. Is username part of pass criteria for the test? If so, that is misleading because there is already PASS or FAIL above it. And if not, let's not log it at all.

> LayoutTests/http/tests/xmlhttprequest/digest-auth.html:9
> +    /*
> +     * If the request contains credentials in its url, it should be stripped from it.
> +     * Also first attempt shouldn't contain basic auth header
> +     */

This should be visible in test output, no need to hide it in a comment.
Comment 5 Basuke Suzuki 2018-09-05 10:23:15 PDT
Comment on attachment 348875 [details]
PATCH

View in context: https://bugs.webkit.org/attachment.cgi?id=348875&action=review

Thanks for reviewing.

>> LayoutTests/http/tests/resources/digest-auth/digest.php:1
>> +<?php
> 
> Calculating a digest response in PHP is quite cunning! However, I can't help but wonder if this would be easier to set up with an Apache config.

PHP is more universal than configuring apache with configuration. Each platform can share same result without configuration. It is very simple script to read.
On the other hand, aligning each port to enable new feature by configuration is harder and require lots of work. Simpler choice seems better to me.

>> LayoutTests/http/tests/xmlhttprequest/digest-auth-expected.txt:2
>> +User: everybody
> 
> This is quite confusing. Is username part of pass criteria for the test? If so, that is misleading because there is already PASS or FAIL above it. And if not, let's not log it at all.

Right. I'll move that to validation check.

>> LayoutTests/http/tests/xmlhttprequest/digest-auth.html:9
>> +     */
> 
> This should be visible in test output, no need to hide it in a comment.

Got it.
Comment 6 Basuke Suzuki 2018-09-05 10:33:26 PDT
Created attachment 348936 [details]
PATCH
Comment 7 Alexey Proskuryakov 2018-09-05 13:50:03 PDT
(In reply to Basuke Suzuki from comment #5)
> PHP is more universal than configuring apache with configuration. Each
> platform can share same result without configuration. It is very simple
> script to read.
> On the other hand, aligning each port to enable new feature by configuration
> is harder and require lots of work. Simpler choice seems better to me.

The downside is that we'll be testing adherence to what this script does, not to what an actual web server does. It's probably OK in this case, but less elegant in my opinion.

Anyway, I won't object to getting it landed in this form.
Comment 8 Basuke Suzuki 2018-09-05 15:34:57 PDT
(In reply to Alexey Proskuryakov from comment #7)
> (In reply to Basuke Suzuki from comment #5)
> > PHP is more universal than configuring apache with configuration. Each
> > platform can share same result without configuration. It is very simple
> > script to read.
> > On the other hand, aligning each port to enable new feature by configuration
> > is harder and require lots of work. Simpler choice seems better to me.
> 
> The downside is that we'll be testing adherence to what this script does,
> not to what an actual web server does. It's probably OK in this case, but
> less elegant in my opinion.
> 
> Anyway, I won't object to getting it landed in this form.

Can you give this r+?
Comment 9 Alex Christensen 2018-09-07 17:12:24 PDT
Comment on attachment 348936 [details]
PATCH

This script is or should be what an actual server does. r=me
Comment 10 WebKit Commit Bot 2018-09-07 17:36:15 PDT
Comment on attachment 348936 [details]
PATCH

Clearing flags on attachment: 348936

Committed r235822: <https://trac.webkit.org/changeset/235822>
Comment 11 WebKit Commit Bot 2018-09-07 17:36:17 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2018-09-07 17:37:24 PDT
<rdar://problem/44246071>
Comment 13 Basuke Suzuki 2018-09-07 21:04:29 PDT
(In reply to Alex Christensen from comment #9)
> Comment on attachment 348936 [details]
> PATCH
> 
> This script is or should be what an actual server does. r=me

Thanks!