Bug 160913 - URLParser should parse URLs without credentials
Summary: URLParser should parse URLs without credentials
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-16 14:55 PDT by Alex Christensen
Modified: 2016-08-16 17:43 PDT (History)
1 user (show)

See Also:


Attachments
Patch (20.51 KB, patch)
2016-08-16 15:03 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (21.00 KB, patch)
2016-08-16 17:09 PDT, Alex Christensen
beidson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2016-08-16 14:55:55 PDT
URLParser should parse URLs without credentials
Comment 1 Alex Christensen 2016-08-16 15:03:11 PDT
Created attachment 286209 [details]
Patch
Comment 2 Brady Eidson 2016-08-16 16:53:39 PDT
Comment on attachment 286209 [details]
Patch

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

> Source/WebCore/platform/URLParser.cpp:60
> +    while (isC0ControlOrSpace(*c))
>          ++c;

From a readability standpoint, this seems dangerous - Like you'd easily walk "c" off the end.

Why is this okay?

> Source/WebCore/platform/URLParser.cpp:423
> +    LOG(URLParser, "%d %d %d %d %d %d %d %d %d %d %d %d %s\n%d %d %d %d %d %d %d %d %d %d %d %d %s",
> +        a.m_isValid, a.m_protocolIsInHTTPFamily, a.m_schemeEnd, a.m_userStart, a.m_userEnd, a.m_passwordEnd, a.m_hostEnd, a.m_portEnd, a.m_pathAfterLastSlash, a.m_pathEnd, a.m_queryEnd, a.m_fragmentEnd, a.m_string.utf8().data(),
> +        b.m_isValid, b.m_protocolIsInHTTPFamily, b.m_schemeEnd, b.m_userStart, b.m_userEnd, b.m_passwordEnd, b.m_hostEnd, b.m_portEnd, b.m_pathAfterLastSlash, b.m_pathEnd, b.m_queryEnd, b.m_fragmentEnd, b.m_string.utf8().data());
> +

Debug-only code, I know. Makes me sad anyways.
Comment 3 Alex Christensen 2016-08-16 17:09:26 PDT
Created attachment 286236 [details]
Patch
Comment 4 Alex Christensen 2016-08-16 17:10:27 PDT
(In reply to comment #2)
> Comment on attachment 286209 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=286209&action=review
> 
> > Source/WebCore/platform/URLParser.cpp:60
> > +    while (isC0ControlOrSpace(*c))
> >          ++c;
> 
> From a readability standpoint, this seems dangerous - Like you'd easily walk
> "c" off the end.
> 
> Why is this okay?
This is not ok, and even though I thought it was not in the scope of this patch, I fixed it and added a test.
> 
> > Source/WebCore/platform/URLParser.cpp:423
> > +    LOG(URLParser, "%d %d %d %d %d %d %d %d %d %d %d %d %s\n%d %d %d %d %d %d %d %d %d %d %d %d %s",
> > +        a.m_isValid, a.m_protocolIsInHTTPFamily, a.m_schemeEnd, a.m_userStart, a.m_userEnd, a.m_passwordEnd, a.m_hostEnd, a.m_portEnd, a.m_pathAfterLastSlash, a.m_pathEnd, a.m_queryEnd, a.m_fragmentEnd, a.m_string.utf8().data(),
> > +        b.m_isValid, b.m_protocolIsInHTTPFamily, b.m_schemeEnd, b.m_userStart, b.m_userEnd, b.m_passwordEnd, b.m_hostEnd, b.m_portEnd, b.m_pathAfterLastSlash, b.m_pathEnd, b.m_queryEnd, b.m_fragmentEnd, b.m_string.utf8().data());
> > +
> 
> Debug-only code, I know. Makes me sad anyways.
This makes me happy.  I put them each on their own line, if that makes you happier.
Comment 5 Alex Christensen 2016-08-16 17:43:20 PDT
http://trac.webkit.org/changeset/204544