Skip to content
New issue

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

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

Already on GitHub? # to your account

Added some new tests for cases that fail. #30

Merged
merged 4 commits into from
Jan 9, 2018

Conversation

keskival
Copy link

These cases are not correctly processed.

@jfromaniello jfromaniello merged commit 8c06ce5 into jfromaniello:master Jan 9, 2018
@jfromaniello
Copy link
Owner

Thanks and sorry for the delay. I published this as url-join@2.0.3.

jfromaniello added a commit that referenced this pull request Jan 10, 2018
@jfromaniello
Copy link
Owner

Heads up.. I had to rever this PR because it breaks other stuff and usage, oops Sorry!.

Master has the tests you added with .skip because I still think is important to solve these

@keskival
Copy link
Author

Thanks anyway! The problems are because my heuristics assume the first part should contain the protocol part, and detects that with ":". One way to fix is to add a condition of no slashes or colons before the assumed protocol colon.
All this is a bit ambiguous, because what we are doing is basically applying heuristics to fix malformed parameters. One thing we must not do is to modify legal URLs. There should be a larger set of tests to make sure legal inputs are not being modified, but are simply concatenated.

@keskival
Copy link
Author

@jfromaniello : Fixed the discovered issue here: #35

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

Successfully merging this pull request may close these issues.

2 participants