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

add HTTP referer header for resource requests #400

Merged
merged 3 commits into from
Aug 16, 2024
Merged

Conversation

rakhnin
Copy link
Contributor

@rakhnin rakhnin commented Aug 15, 2024

to mimic more-browser-like-behavior on loading page resources. Sometimes their response depends on referer header.

e.g. I faced with the problem to load font data for page, because it was protected for "direct" loading, got 401, but with the referer header it looks like request from browser.

Copy link
Member

@snshn snshn left a comment

Choose a reason for hiding this comment

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

This is really cool, I haven't thought of that, thank you. And please feel free to add yourself as one of the authors of the program, if you'd like some well-deserved open-source fame.

src/url.rs Outdated
@@ -79,6 +79,17 @@ pub fn parse_data_url(url: &Url) -> (String, String, Vec<u8>) {
(media_type, charset, blob)
}

pub fn referer_url(url: Url) -> Url {
let mut url = url.clone();
// https://httpwg.org/specs/rfc9110.html#field.referer
Copy link
Member

Choose a reason for hiding this comment

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

I think putting "Spec: " or "Reference: " before the URL could be a good way to make it more readable, otherwise it's just a commented out URL. I try not to put them myself, since URLs tend to become 404/301 sometimes, and then it'll have to be updated.

There's this file here https://github.com/Y2Z/monolith/blob/master/docs/references.md that I created to keep all used references, maybe we could move it there, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just add a Spec: prefix.

It's nice to have internal documentation and refer to it, but in case when ref leads just to another/external ref, in my vision, it doesn't make sense. Anyway in case external link "died" one have to update it no matter its place in the source code.

src/url.rs Outdated
pub fn referer_url(url: Url) -> Url {
let mut url = url.clone();
// https://httpwg.org/specs/rfc9110.html#field.referer
// MUST NOT include the fragment and userinfo components of the URI
Copy link
Member

Choose a reason for hiding this comment

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

"MUST NOT" sounds a bit too commanding, as if somebody's shouting at you. "Must not" would be nicer and easier to read.

src/url.rs Outdated
@@ -79,6 +79,17 @@ pub fn parse_data_url(url: &Url) -> (String, String, Vec<u8>) {
(media_type, charset, blob)
}

pub fn referer_url(url: Url) -> Url {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe something like get_referer_url, format_referer_url, or url_to_referer_url would be more descriptive? Just to be more consistent with function names in that file.

@@ -46,7 +46,7 @@ mod passing {
}

#[test]
fn removesempty_fragment_and_keeps_empty_query() {
fn removes_empty_fragment_and_keeps_query() {
Copy link
Member

Choose a reason for hiding this comment

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

Embarrassing, but typos aren't as shameful when they're in tests 😄


#[test]
fn preserve_original() {
let u: Url = Url::parse("https://somewhere.com/font.eot#iefix").unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

nit: naming it url and referer_url might make it a bit easier to read for whoever sees it for the first time, u can be "user"

@rakhnin
Copy link
Contributor Author

rakhnin commented Aug 16, 2024

And please feel free to add yourself as one of the authors of the program, if you'd like some well-deserved open-source fame.

Nice to have it ;), thanks

@snshn snshn merged commit f151a33 into Y2Z:master Aug 16, 2024
9 checks passed
@rakhnin rakhnin deleted the referer branch August 16, 2024 15:50
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.

2 participants