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

Regression in URI.join #83

Open
casperisfine opened this issue Jun 24, 2023 · 2 comments
Open

Regression in URI.join #83

casperisfine opened this issue Jun 24, 2023 · 2 comments

Comments

@casperisfine
Copy link

The logic of this code is questionable, but it has worked for a while and the behavior seem to have changed in the last couple weeks:

puts URI.join("http://example.com/", "//", "path")

3.2 and older:

http://example.com/path

3.3.0-dev:

http:///path

I'll try to figure out what caused this and provide a fix.

@casperisfine
Copy link
Author

Bisecting show that c1d1757 is what introduced the failure.

FYI @nobu

nobu added a commit to nobu/uri that referenced this issue Jun 25, 2023
In relative referece, host part can be ommitted but can not be empty.
nobu added a commit to nobu/uri that referenced this issue Jun 25, 2023
nobu added a commit to nobu/uri that referenced this issue Jun 25, 2023
In relative referece, host part can be ommitted but can not be empty.
@casperisfine
Copy link
Author

I see you worked on it in 6b64023.

To be honest, reviewing our code, File.join is a much better method for what the author wanted to do.

I don't think the new behavior of URI.join is necessarily wrong, if you consider its implementation. It's just that the name is a bit confusing. join suggest nothing is lost, URI.join behaves more like a Hash#merge than File.join or Array#join.

I''m convinced that 99% of the time, users actually want File.join.

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

No branches or pull requests

1 participant