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

Undefined offset notice #8

Closed
sharifzadesina opened this issue Dec 15, 2016 · 5 comments
Closed

Undefined offset notice #8

sharifzadesina opened this issue Dec 15, 2016 · 5 comments
Assignees

Comments

@sharifzadesina
Copy link

Hi
I have tested this example in README.md file. the result is ok but there is a notice.
screenshot from 2016-12-15 19-40-02

@sharifzadesina
Copy link
Author

@harikt I think this should be fix and I am sure it is not so hard to fixing this, is it? ;)

@harikt
Copy link
Member

harikt commented Dec 26, 2016

@sharifzadesina I wonder whether that was a typo in the docs ( have not put the additional star ) . So application/xml;q=1.0,text/csv;q=0.5,*;q=0.1 will become application/xml;q=1.0,text/csv;q=0.5,*/*;q=0.1 . I have a fix if the doc is correct. But need more research before I merge the same.

Below is the fix for the same

class MediaValue ....

    protected function init()
    {
        // list($this->type, $this->subtype) = explode('/', $this->value);
        $values = explode('/', $this->value);
        $this->type = $values[0];
        $this->subtype = isset($values[1]) ? $values[1] : '*';
    }

@pmjones any different thought?

harikt added a commit to harikt/Aura.Accept that referenced this issue Dec 26, 2016
@sharifzadesina
Copy link
Author

sharifzadesina commented Dec 26, 2016

@harikt You are right according to the RFC7231 this should be */*. but unfortunately chrome sends *.
this is not cause of doc dude, this is cause of chrome which doesn't follow standard syntax. I am not sure but it seems this bug was fixed in new version of chrome.

@harikt
Copy link
Member

harikt commented Dec 26, 2016

Thank you @sharifzadesina for linking to the RFC.

For reference to others : fixed via #9 . In case of better fixes send a different one.

Thank you.

pmjones pushed a commit that referenced this issue Dec 26, 2016
Add a test case and fix for issue #8 reporting warning
@pmjones
Copy link
Member

pmjones commented Dec 26, 2016

Looks good @harikt ! Thanks, @sharifzadesina , for the report.

Fixed in #9

@pmjones pmjones closed this as completed Dec 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants