Bug 80265 - The path of URL has redundant slashes should not be cached again in offline web applications
Summary: The path of URL has redundant slashes should not be cached again in offline w...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-05 04:26 PST by huangxueqing
Modified: 2012-03-05 23:21 PST (History)
5 users (show)

See Also:


Attachments
test case (274 bytes, application/octet-stream)
2012-03-05 04:26 PST, huangxueqing
no flags Details
test case (274 bytes, text/html)
2012-03-05 04:42 PST, huangxueqing
no flags Details
patch (17.36 KB, patch)
2012-03-05 05:22 PST, huangxueqing
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
skip layout test cases in chromium platform (18.24 KB, patch)
2012-03-05 06:25 PST, huangxueqing
buildbot: commit-queue-
Details | Formatted Diff | Diff
patch (18.47 KB, patch)
2012-03-05 07:00 PST, huangxueqing
buildbot: commit-queue-
Details | Formatted Diff | Diff
patch (18.46 KB, patch)
2012-03-05 07:45 PST, huangxueqing
no flags Details | Formatted Diff | Diff
patch (18.49 KB, patch)
2012-03-05 07:51 PST, huangxueqing
abarth: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description huangxueqing 2012-03-05 04:26:02 PST
Created attachment 130093 [details]
test case

INTRODUCTION:
It's commom that user input more than one slashes in path of url due to careless, such as "http://127.0.0.1:8000//appcache//test.html" actually specify "http://127.0.0.1:8000/appcache/test.html".

REPOS STEPS:
1. Unzip attachment and move "appcache" folder into %APACHE_ROOT%;
2. Navigate http://127.0.0.1/appcache/test.html;
2. Naviagte http://127.0.0.1/////appcache/////test.html;

ACTUAL RESULTS:
There ware two record in cacheGroup table identified by "http://127.0.0.1/appcache/test.manifest" and "http://127.0.0.1/////appcache/////test.manifest" respectively.

EXPECTED RESULTS:
Only one record in cacheGroup identified by "http://127.0.0.1/appcache/test.manifest".



REPOS STEPS:
1. Navigate http://127.0.0.1/appcache/test.html;
2. Shutdown apache;
3. Navigate http://127.0.0.1/////appcache/test.html;

ACTUAL RESULTS:
Failed to load test.html from local cache.

EXPECTED RESULTS:
Load test.html from local cache correctly.
Comment 1 WebKit Review Bot 2012-03-05 04:29:45 PST
Attachment 130093 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1
Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 2 huangxueqing 2012-03-05 04:42:17 PST
Created attachment 130094 [details]
test case
Comment 3 huangxueqing 2012-03-05 05:22:16 PST
Created attachment 130099 [details]
patch
Comment 4 WebKit Review Bot 2012-03-05 05:54:21 PST
Comment on attachment 130099 [details]
patch

Attachment 130099 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11803984

New failing tests:
http/tests/appcache/fallback-multiply-slashes.html
http/tests/appcache/appcache-multiply-slashes.html
Comment 5 huangxueqing 2012-03-05 06:25:09 PST
Created attachment 130112 [details]
skip layout test cases in chromium platform
Comment 6 Build Bot 2012-03-05 06:34:29 PST
Comment on attachment 130112 [details]
skip layout test cases in chromium platform

Attachment 130112 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/11808564
Comment 7 Build Bot 2012-03-05 06:42:42 PST
Comment on attachment 130099 [details]
patch

Attachment 130099 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/11809382
Comment 8 huangxueqing 2012-03-05 07:00:59 PST
Created attachment 130123 [details]
patch
Comment 9 Build Bot 2012-03-05 07:33:18 PST
Comment on attachment 130123 [details]
patch

Attachment 130123 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/11809409
Comment 10 huangxueqing 2012-03-05 07:45:59 PST
Created attachment 130131 [details]
patch
Comment 11 huangxueqing 2012-03-05 07:51:55 PST
Created attachment 130134 [details]
patch
Comment 12 Adam Barth 2012-03-05 10:08:00 PST
Comment on attachment 130134 [details]
patch

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

> Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp:59
> +String stripRedundantSlashes(const String& str)

I'm sorry that I will not be able to review your patch fully, but this function clearly does not belong in this file.
Comment 13 Alexey Proskuryakov 2012-03-05 11:11:52 PST
I don't think that there is a concept of normalizing a URL by removing duplicate slashes, is there?

Thus, this looks like correct behavior to me.
Comment 14 Adam Barth 2012-03-05 11:14:48 PST
> I don't think that there is a concept of normalizing a URL by removing duplicate slashes, is there?

Correct.  I'm not away of any precedent for normalizing URIs by removing redundant slashes from the path.  There is some precedent for removing redundant slashes between the scheme and the authority, but even that is controversial (e.g., in the IETF's IRI working group).
Comment 15 Alexey Proskuryakov 2012-03-05 11:28:58 PST
Please feel free to reopen if there is evidence that this is a bug.
Comment 16 huangxueqing 2012-03-05 17:43:57 PST
Thanks for review. It's not well to put this function into KURL.h since KURL never call it. Any suggestions?  

(In reply to comment #12)
> (From update of attachment 130134 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130134&action=review
> 
> > Source/WebCore/loader/appcache/ApplicationCacheGroup.cpp:59
> > +String stripRedundantSlashes(const String& str)
> 
> I'm sorry that I will not be able to review your patch fully, but this function clearly does not belong in this file.
Comment 17 huangxueqing 2012-03-05 17:54:29 PST
No, but in my view, "appcache" as a "nightly web server" in local since it provide  resource just like a web server, and web server always handle redundant slashes in path correctly such as apache, thus "appcache" should do that also.
In addition, it's unnecessary that store a manifest twice.

(In reply to comment #13)
> I don't think that there is a concept of normalizing a URL by removing duplicate slashes, is there?
> 
> Thus, this looks like correct behavior to me.
Comment 18 Alexey Proskuryakov 2012-03-05 19:56:23 PST
> No, but in my view, "appcache" as a "nightly web server"

It's perfectly appropriate for a Web server to provide different responses to requests with different number of slashes.
Comment 19 huangxueqing 2012-03-05 23:21:07 PST
(In reply to comment #18)
> > No, but in my view, "appcache" as a "nightly web server"
> 
> It's perfectly appropriate for a Web server to provide different responses to requests with different number of slashes.

All right, thanks for review. Maybe I will investigate how web server handle it.