Site Metadata: resolve relative URLs for embedded images/videos #3338
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
With this change we opt into next (non breaking) versions of webpage-rs
Nutomic
reviewed
Jun 26, 2023
| let og_embed_url = page | ||
| .opengraph | ||
| .videos | ||
| .first() | ||
| .and_then(|v| Url::parse(&v.url).ok()); | ||
| .and_then(|v| url.join(&v.url).ok()); |
There was a problem hiding this comment.
This would break if a site links to an external domain for images/videos. Not sure how common that is, but it could already happen in case of using cdn.example.com for media. So this should probably be conditional, and only use join if url.domain() is None (or maybe theres a better way to check it).
There was a problem hiding this comment.
Perhaps surprisingly, this works fine. join on an absolute URL will result in that URL.
I have added a comment and a test for this.
Nutomic
approved these changes
Jun 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey I'm the maintainer of the
webpagecrate you are using to fetch metadata about submitted urls.An issue was raised orottier/webpage-rs#8 (comment) about relative URLs which I think should be fixed on your end.
This pull request contains the fix, and I also changed the dependency to opt into future 1.x versions of the lib. The linked issue contains some other stuff that I will probably fix in the 1.6+ versions.