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

Origin Header #651

Closed
fuchsnj opened this issue Sep 9, 2015 · 13 comments
Closed

Origin Header #651

fuchsnj opened this issue Sep 9, 2015 · 13 comments
Labels
A-headers Area: headers. E-easy Effort: easy. A task that would be a great starting point for a new contributor.

Comments

@fuchsnj
Copy link
Contributor

fuchsnj commented Sep 9, 2015

It seems like the "Origin" header is missing here

http://tools.ietf.org/id/draft-abarth-origin-03.html

@seanmonstar
Copy link
Member

Is this in common use? I've never used it myself, and when checking the dev consoles, I notice neither Firefox nor Chrome set that header while browsing.

@untitaker
Copy link
Contributor

The Origin header is set for CORS requests.

On 15 October 2015 01:36:27 CEST, Sean McArthur notifications@github.com wrote:

Is this in common use? I've never used it myself, and when checking the
dev consoles, I notice neither Firefox nor Chrome set that header while
browsing.


Reply to this email directly or view it on GitHub:
#651 (comment)

Sent from my phone. Please excuse my brevity.

@seanmonstar
Copy link
Member

Ah yes, quite true. I had forgotten.

@seanmonstar seanmonstar added A-headers Area: headers. E-easy Effort: easy. A task that would be a great starting point for a new contributor. labels Oct 15, 2015
@mfeckie
Copy link
Contributor

mfeckie commented Nov 19, 2015

@seanmonstar Is anyone working on this? If not I'd be interested in giving it a go

@seanmonstar
Copy link
Member

Go for it!

On Thu, Nov 19, 2015 at 4:56 AM Martin Feckie notifications@github.com
wrote:

@seanmonstar https://github.com/seanmonstar Is anyone working on this?
If not I'd be interested in giving it a go


Reply to this email directly or view it on GitHub
#651 (comment).

@untitaker
Copy link
Contributor

I don't think anybody is working on this.

On Thu, Nov 19, 2015 at 04:56:13AM -0800, Martin Feckie wrote:

@seanmonstar Is anyone working on this? If not I'd be interested in giving it a go


Reply to this email directly or view it on GitHub:
#651 (comment)

@puhrez
Copy link
Contributor

puhrez commented Jul 4, 2016

Would love to get newly-rusty hands wet; what's going on here?

A variety of PRs have been closed, and one (to another project?)

@untitaker
Copy link
Contributor

I think the main discussion is under #691, basically we waited until rust-url was 1.0 (where its Origin type was rewritten) and then forgot about it.

On 5 July 2016 01:21:00 EEST, Michael Perez notifications@github.com wrote:

Would love to get newly-rusty hands wet; what's going on here?

A variety of PRs have been closed, and one (to another project?)


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#651 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@seanmonstar
Copy link
Member

The current type in rust-url is an enum with an OpaqueOrigin variant, which is not needed in the header. So, I'm thinking we do something like this:

pub struct Origin(String, String, Option<u16>);

// support default ports
impl PartialEq for Origin {
    fn eq(&self, other: &Origin) -> bool {
        if self.0 == other.0 && self.1 == other.1 {
            self.2.or_else(|| default_port(&self.0)) == other.2.or_else(|| default_port(&other.0))
        }
        false
    }
}

fn default_port(scheme: &str) -> Option<u16> {
    match scheme {
        "http" => Some(80),
        "https" => Some(443),
        _ => None,
    }
}

@puhrez
Copy link
Contributor

puhrez commented Jul 6, 2016

@seanmonstar that seems good for a single "serialized-origin". I'm not very familiar with things (in general) but according to the spec the value of an Origin header may actually be a vec of this implementation of Origin.

Aside from that in order to parse/generate, after having read the docs the Origin struct also needs to implement Header and HeaderFormat right?

also aside: y use u16 when there are no negative ports?

@seanmonstar
Copy link
Member

@puhrez since the Origin header only makes in CORS requests, may as well use the definition from the most up-to-date spec governing them: https://fetch.spec.whatwg.org/#origin-header

In that spec, the Origin is only ever 1 value.

Yes, a header needs to implement Header (and HeaderFormat in the 0.9x branch. If possible, the header! macro handles this merge of traits).

A u16 means unsigned, so a bit isn't dedicated to the sign. This means it's range is 0-65535, which is the full range of possible ports.

@puhrez
Copy link
Contributor

puhrez commented Jul 6, 2016

Ah yes, I knew it meant unsigned but for some reason (not being very used to u/i distinctions), I mixed the concepts.

re header! macro trait merging:
the idea then becomes to do something like (from the docs)

header! { (Origin, "Origin") => [String, String, Option[u16]] }
<insert yr impl block and default_port fn here>

UPDATE:
actually... reading thru more of the doc,

"This works well for simple "string" headers. But the header system actually involves 2 parts: parsing, and formatting. If you need to customize either part, you can do so."

since the optional port is not a string, do we have to implement the Header/HeaderFormat trait anyway? regardless, I think would this be an appropriate place to use url::Host for the host like rust-url in its definition so we get all that typed goodness.

on second though, perhaps it can use header::Host since that already has a sense of hostname and port. Moreover, when that gets moved over to use url::Host, this one naturally benefits.

on third thought, isn't Origin (according to the newest spec) just an optional header::Header::Host with a scheme?

@puhrez
Copy link
Contributor

puhrez commented Jul 6, 2016

started a pull request incorporating my comments above as well as trying to conform to contribution style, haven't tested it well (read: at all) yet tho :).

used header::Host for inspiration

seanmonstar pushed a commit that referenced this issue Jul 12, 2016
Add an Origin header so users may properly send CORS requests

Closes #651
seanmonstar pushed a commit that referenced this issue Jul 13, 2016
Add an Origin header so users may properly send CORS requests

Closes #651
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-headers Area: headers. E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
None yet
Development

No branches or pull requests

5 participants