Bug 161443 - URLParser should handle . and .. in URL paths
Summary: URLParser should handle . and .. in URL paths
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-31 11:23 PDT by Alex Christensen
Modified: 2016-09-01 13:35 PDT (History)
2 users (show)

See Also:


Attachments
Patch (16.21 KB, patch)
2016-08-31 11:23 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (16.21 KB, patch)
2016-08-31 14:27 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (16.26 KB, patch)
2016-09-01 11:56 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (16.40 KB, patch)
2016-09-01 12:47 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-31 11:23:31 PDT
URLParser should handle . and .. in URL paths
Comment 1 Alex Christensen 2016-08-31 11:23:58 PDT
Created attachment 287522 [details]
Patch
Comment 2 WebKit Commit Bot 2016-08-31 11:40:48 PDT
Attachment 287522 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Alex Christensen 2016-08-31 14:27:31 PDT
Created attachment 287543 [details]
Patch
Comment 4 Alex Christensen 2016-09-01 11:56:34 PDT
Created attachment 287649 [details]
Patch
Comment 5 Brady Eidson 2016-09-01 12:07:29 PDT
Comment on attachment 287543 [details]
Patch

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

> Source/WebCore/platform/URLParser.cpp:152
> +static bool isSingleDotPathSegment(StringView::CodePoints::Iterator c, const StringView::CodePoints::Iterator& end)
> +{
> +    if (c == end)
> +        return false;
> +    if (*c == '.') {
> +        ++c;
> +        return c == end || *c == '/' || *c == '\\' || *c == '?' || *c == '#';
> +    }
> +    if (*c != '%')
> +        return false;
> +    ++c;
> +    if (c == end || *c != '2')
> +        return false;
> +    ++c;
> +    if (c == end)
> +        return false;
> +    if (*c == 'e' || *c == 'E') {
> +        ++c;
> +        return c == end || *c == '/' || *c == '\\' || *c == '?' || *c == '#';
> +    }
> +    return false;
> +}

"2E" is a magic constant that should be more obvious.

static const char dotASCIICode = "2E";
?

> Source/WebCore/platform/URLParser.cpp:171
> +    if (c == end || *c != '2')
> +        return false;
> +    ++c;
> +    if (c == end)
> +        return false;
> +    if (*c == 'e' || *c == 'E') {
> +        ++c;

Same magic constant comment.

> Source/WebCore/platform/URLParser.cpp:194
> +        ASSERT(*c == '%');
> +        ++c;
> +        ASSERT(*c == '2');
> +        ++c;
> +        ASSERT(*c == 'e' || *c == 'E');
> +        ++c;

Same comment you guessed it.

> Source/WebCore/platform/URLParser.cpp:215
> +        ASSERT(*c == '2');
> +        ++c;
> +        ASSERT(*c == 'e' || *c == 'E');
> +        ++c;
> +    }

You guessed it.
Comment 6 Alex Christensen 2016-09-01 12:47:20 PDT
Created attachment 287657 [details]
Patch
Comment 7 Alex Christensen 2016-09-01 13:35:40 PDT
http://trac.webkit.org/changeset/205312