[Webkit-unassigned] [Bug 165521] Add Link header support for preload.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=165521

--- Comment #21 from Alex Christensen <achristensen at apple.com> ---
(In reply to comment #17)
> >> Source/WebCore/loader/LinkHeader.cpp:122
> >> +    ASSERT(position <= end);
> > 
> > This assertion should be at every function that has a position and an end.
> > It seems like an iterator type would be better than raw pointers in this whole patch.  StringView.h has code point and code unit iterators.
> 
> Good point regarding the assertion. Added everywhere else.
> 
> Regarding switching to StringView's iterators I tried to convert the logic
> to that, but it seems to be missing some concepts needed here (skipWhile,
> skipExactly and/or getting an iterator for a position in the middle of a
> StringView).
> For now I kept it using pointers, but if we want to go the iterator route,
> it'd require adding a new iterator type (e.g. ParserStringView) which will
> include the required concepts. Let me know if you think this is the route we
> should take and if so, where should such a class be located.
Yeah, it's probably not worth adding another iterator type just for this.
> >> Source/WebCore/loader/LinkHeader.cpp:274
> >> +    else if (name == LinkParameterAnchor)
> > 
> > It looks like a switch would be better here.
> 
> switched
lol
> >> LayoutTests/http/tests/preload/resources/download_resources_from_header.php:2
> >> +header("Link: <../resources/dummy.js>; rel=preload; as=script", false);
> > 
> > Can we put a complete absolute URL here?  We should add a test with a URL that looks like http://localhost:8000/preload/resources/something.html on a page with CSP that doesn't allow loading from localhost.  We should also add a test with an invalid URL like http://host:invalidport/ just to see what happens.  It's hard to make a URL fail to parse when only giving the path.  A fragment-only URL might also be interesting. Also a lowercase "link", a missing colon, no <> or URL, <> but no characters inside it, and a missing space after the colon.  Also what about emojis, invalid surrogate pairs, and null characters in the middle of the URL?
> 
> Added tests for CSP blocked, invalid port, fragment-only, lower-case link,
> missing colon, missing spaces, missing URL, etc.
> (Invalid port passes the isPreloaded() test, but I'm guessing that it's
> being blocked further down the stack)
> 
> I'm not sure how to add emojis, invalid surrogate pairs and null chars in
> PHP... Any examples you can point me to?
Now that I think about it, HTTP headers can only contain 8-bit values, so we can't have 16-bit characters in an HTTP header.

EWS failure looks unrelated.  I'll re-review this soon.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170118/6cce7638/attachment-0001.html>