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

Fix HTTP.rb when using non-ASCII URIs #1370

Merged
merged 2 commits into from
Jan 22, 2025
Merged

Fix HTTP.rb when using non-ASCII URIs #1370

merged 2 commits into from
Jan 22, 2025

Conversation

unflxw
Copy link
Contributor

@unflxw unflxw commented Jan 22, 2025

Follow up to #1369 (thanks @stephannv!)

Fix HTTP.rb non-ascii URI

When Appsignal is enabled, it hooks into HTTP request method. It
uses URI.parse when uri is a string, but URI.parse doesn't
support non-ascii characters:

HTTP.get("http://www.example.com/áéíóúãÔù")

With Appsignal enabled, the code above raises this error:

URI::InvalidURIError:
       URI must be ascii only

The solution I found was using Addressable::URI.parse since it
handles non-ascii characters.

ps.: Addressable is a HTTP's runtime dependency since 2015

Use HTTP::URI if defined

The http gem exposes the HTTP::URI module, which on different
versions of the gem either subclasses or includes Addressable::URI.

Use it instead of referring directly to Addressable::URI, avoiding
explicitly depending on the implementation detail that HTTP::URI
uses Addressable::URI internally.

When HTTP::URI is not defined (in http gem versions before 0.8.0)
fall back to using URI instead.

@backlog-helper
Copy link

backlog-helper bot commented Jan 22, 2025

✔️ All good!

New issue guide | Backlog management | Rules | Feedback

When Appsignal is enabled, it hooks into HTTP request method. It
uses `URI.parse` when uri is a string, but `URI.parse` doesn't
support non-ascii characters:

```ruby
HTTP.get("http://www.example.com/áéíóúãÔù")
```

With Appsignal enabled, the code above raises this error:

```
URI::InvalidURIError:
       URI must be ascii only
```

The solution I found was using Addressable::URI.parse since it
handles non-ascii characters.

ps.: Addressable is a HTTP's runtime dependency since 2015
@unflxw unflxw force-pushed the fix-http-non-ascii-uri branch from b1e6457 to 75592ac Compare January 22, 2025 07:14
Copy link
Member

@tombruijn tombruijn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR and thanks Noemi for the tweaks to it :)
Please also add a changeset

The `http` gem exposes the `HTTP::URI` module, which on different
versions of the gem either subclasses or extends `Addressable::URI`.

Use it instead of referring directly to `Addressable::URI`, avoiding
explicitly depending on the implementation detail that `HTTP::URI`
uses `Addressable::URI` internally.

When `HTTP::URI` is not defined (in `http` gem versions before 0.8.0)
fall back to using `URI` instead.
@unflxw unflxw force-pushed the fix-http-non-ascii-uri branch from 75592ac to fce0acd Compare January 22, 2025 09:20
@unflxw unflxw merged commit 2fd989f into main Jan 22, 2025
157 checks passed
@unflxw unflxw changed the title Fix HTTP.rb when using non-ascii URIs Fix HTTP.rb when using non-ASCII URIs Jan 22, 2025
@backlog-helper
Copy link

  • This Pull Request has been closed or merged, but is still in a column that is considered to be 'in progress'. Please move the Pull Request to the 'Done' column when no more work will be done on this. - (More info)

This is a message from the daily scheduled checks.

New issue guide | Backlog management | Rules | Feedback

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

Successfully merging this pull request may close these issues.

4 participants