Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Support for Origin header #5029

Closed
wants to merge 4 commits into from
Closed

Support for Origin header #5029

wants to merge 4 commits into from

Conversation

jas-
Copy link

@jas- jas- commented Aug 26, 2013

Per issue #5025 discussed with @weierophinney. Please let me know if I missed a step as I followed the contributing guide.

@macnibblet
Copy link
Contributor

@jas- You got a commit that does not belong here

@jas-
Copy link
Author

jas- commented Aug 26, 2013

@macnibblet Whoops, thanks!

@pauloelr
Copy link
Contributor

@jas- seems like you create your branch based on this PR #5008 if that is the case the easiest way i see to correct it is create a new branch based on master and copy the new two files to that branch and edit the other file again.

Or merge or rebase your branch with an updated version of master (Since the PR has already been acepted)

throw new Exception\InvalidArgumentException('Invalid header line for Origin string: "' . $name . '"');
}

if (!filter_var($value, FILTER_VALIDATE_URL)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem with filter_var is that it will return false for "example.com". iirc I've seen many CORS requests where the Origin is not using the schema.

Copy link
Author

Choose a reason for hiding this comment

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

According to the ietf working draft the UA behavior should be conforming to RFC 3986 regarding the proper URI format which (without more testing on my own) this doc seems to support your claim. Are you aware of a sure fire RFC 3986 regex to take its place? I will have to test this a bit but I found one that seems to conform on stack overflow

preg_match(/^(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?/, $value)

Copy link
Contributor

Choose a reason for hiding this comment

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

What about using Zend\Uri validator ?

@@ -56,6 +56,7 @@ class HeaderLoader extends PluginClassLoader
'lastmodified' => 'Zend\Http\Header\LastModified',
'location' => 'Zend\Http\Header\Location',
'maxforwards' => 'Zend\Http\Header\MaxForwards',
'Origin' => 'Zend\Http\Header\Origin',
Copy link
Member

Choose a reason for hiding this comment

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

lowercase keys here are necessary. I can do that on merge, though.

weierophinney added a commit that referenced this pull request Oct 22, 2013
Support for Origin header
weierophinney added a commit that referenced this pull request Oct 22, 2013
- lowercase "origin" in plugin loader
- remove unnecessary lines in test class
weierophinney added a commit that referenced this pull request Oct 22, 2013
@ghost ghost assigned weierophinney Oct 22, 2013
@weierophinney
Copy link
Member

Merged to develop for release with 2.3.0.

@jas-
Copy link
Author

jas- commented Oct 23, 2013

@weierophinney Thanks!

Ocramius added a commit that referenced this pull request Apr 14, 2014
gianarb pushed a commit to zendframework/zend-eventmanager that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-eventmanager that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-servicemanager that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-inputfilter that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-inputfilter that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015
weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015
- lowercase "origin" in plugin loader
- remove unnecessary lines in test class
weierophinney added a commit to zendframework/zend-http that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-http that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-http that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-log that referenced this pull request May 15, 2015
gianarb pushed a commit to zendframework/zend-log that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants