Bug 189286 – [LayoutTests] Add digest access authentication test.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Basuke Suzuki]

There seems no simple Digest authentication test. Add it.

[Basuke Suzuki]

Created attachment 348875 [details]
PATCH

[EWS Watchlist]

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

[EWS Watchlist]

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

[Alexey Proskuryakov]

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.

[Basuke Suzuki]

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.

[Basuke Suzuki]

Created attachment 348936 [details]
PATCH

[Alexey Proskuryakov]

(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.

[Basuke Suzuki]

(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+?

[Alex Christensen]

Comment on attachment 348936 [details]
PATCH

This script is or should be what an actual server does. r=me

[WebKit Commit Bot]

Comment on attachment 348936 [details]
PATCH

Clearing flags on attachment: 348936

Committed r235822: <https://trac.webkit.org/changeset/235822>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44246071>

[Basuke Suzuki]

(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!