Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for several parsing bugs #20

Merged
merged 4 commits into from
Jun 17, 2017
Merged

Conversation

tsibley
Copy link
Contributor

@tsibley tsibley commented Jun 9, 2017

Including a source of runtime errors and denial-of-service via user-controlled regexes.

See the commit messages for more details.

This test case will make the tests appear to hang.  If let to run long
enough, the test should actually pass.  I didn't see a way using
elm-test to set a timeout on a test and ensure that the test case
finishes within that timeout.

In extractPath, the URL authority (aka host as called by Erl) is used
directly in the regex, without escaping, to match against itself for
removal.  The host part is untrusted user-input and shouldn't be used as
a regular expression itself.  A fix is in the next commit.
The return value from extractHost should be used literally, not as a
pattern.  Escape any regex metachars it contains.
The string "//" may appear anywhere in the URL, not just immediately
before the authority (host).

I've tried to cover several similar cases, but there may still be edge
cases I missed.
We can only assume that the URL authority (host) immediately follows
"//" when it's at the beginning of the URL (i.e. in a scheme-less URL).

Furthermore, when using "//" or "://" as a delimiter, we want the
right-hand part from the *left-most* occurrence of the delimiter, not
the right-most occurrence.
@sporto
Copy link
Owner

sporto commented Jun 10, 2017

Thanks, I will have a look soon

@sporto sporto merged commit a8f7185 into sporto:master Jun 17, 2017
@sporto
Copy link
Owner

sporto commented Jun 17, 2017

Thanks

@tsibley tsibley deleted the parsing-bugs branch June 17, 2017 03:15
@tsibley
Copy link
Contributor Author

tsibley commented Nov 9, 2017

Thanks for merging this back in June. Can you release 13.0.1 so that it's usable in projects? The latest on elm-package.org is 13.0.0.

@sporto
Copy link
Owner

sporto commented Nov 12, 2017

Thought I did, thanks for reminding me. Done now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants