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

[6.0] Fix incorrect HTTP OPTION verb #29828

Merged
merged 1 commit into from Sep 2, 2019
Merged

[6.0] Fix incorrect HTTP OPTION verb #29828

merged 1 commit into from Sep 2, 2019

Conversation

nhedger
Copy link
Contributor

@nhedger nhedger commented Sep 2, 2019

Hey there,

As I was testing a new app I'm working on, I stumbled across the option and optionJson methods in Illuminate\Foundation\Testing\Concerns\MakesHttpRequests. I had weird exceptions so I decided to dig a little.

I believe a mistake was introduced in #29258 because the HTTP OPTION method does not exist but the OPTIONS one does.

For reference : https://tools.ietf.org/html/rfc7231#section-4

@nhedger nhedger changed the title Fix incorrect HTTP OPTION verb [6.0] Fix incorrect HTTP OPTION verb Sep 2, 2019
@taylorotwell taylorotwell merged commit 7a9ca97 into laravel:6.0 Sep 2, 2019
@nhedger nhedger deleted the patch-2 branch September 2, 2019 16:48
@nhedger
Copy link
Contributor Author

nhedger commented Sep 2, 2019

@taylorotwell The same error exists on Lumen 5.8. Should I create a new PR or will this get merged into Lumen eventually ? https://github.com/laravel/lumen-framework/blob/5.8/src/Testing/Concerns/MakesHttpRequests.php#L138-L153. Regards.

@driesvints
Copy link
Member

@nhedger best to send to master on that repo

@nhedger
Copy link
Contributor Author

nhedger commented Sep 2, 2019

Will do, thanks.

@GrahamCampbell
Copy link
Member

I guess this error shows nobody uses this code?

@nhedger
Copy link
Contributor Author

nhedger commented Sep 2, 2019

The OPTIONS verb itself does not appear to be heavily used but I guess it cannot do harm to keep it ?

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.

None yet

4 participants