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.

Sign up for GitHub

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

Jump to bottom

[8.x] Fix OPTIONS method bug with use same path and diff domain when cache route #35714

Merged
merged 1 commit into from
Dec 28, 2020
Merged

[8.x] Fix OPTIONS method bug with use same path and diff domain when cache route #35714

merged 1 commit into from
Dec 28, 2020

Conversation

MilesChou
Copy link
Contributor

@MilesChou MilesChou commented Dec 25, 2020 •
edited
Loading

When set the same path and diff domain route and cache route, the first route with OPTIONS method will return 404.

reproduce step

  • PHP version: 7.4
  • Laravel version: v8.5.5
  • Laravel Framework version: v8.20.1
  1. edit routes/web.php file:
Route::domain('foo.localhost')->get('/login', 'FooController@index'); // Will return 404 when use OPTIONS method
Route::domain('bar.localhost')->get('/login', 'BarController@index');

Notice: same path, but different domain.

Route list:

$ php artisan route:list
+---------------+----------+-------+------+------------------------------------------+------------+
| Domain        | Method   | URI   | Name | Action                                   | Middleware |
+---------------+----------+-------+------+------------------------------------------+------------+
|               | GET|HEAD | /     |      | Closure                                  | web        |
| foo.localhost | GET|HEAD | login |      | App\Http\Controllers\FooController@index | web        |
| bar.localhost | GET|HEAD | login |      | App\Http\Controllers\BarController@index | web        |
+---------------+----------+-------+------+------------------------------------------+------------+
  1. cache route
php artisan route:cache
# or 
php artisan optimize
  1. start server and curl route with OPTIONS method:
$ curl -v -X OPTIONS http://foo.localhost:8000/login
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to foo.localhost (127.0.0.1) port 8000 (#0)
> OPTIONS /login HTTP/1.1
> Host: foo.localhost:8000
> User-Agent: curl/7.64.1
> Accept: */*
>
* HTTP 1.0, assume close after body
< HTTP/1.0 404 Not Found
< Host: foo.localhost:8000
< Date: Fri, 25 Dec 2020 08:55:00 GMT
< Connection: close
< X-Powered-By: PHP/7.4.13
< Cache-Control: no-cache, private
< date: Fri, 25 Dec 2020 08:55:00 GMT
< Content-type: text/html; charset=UTF-8
<
<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="utf-8">
        <meta name="viewport" content="width=device-width, initial-scale=1">

        <title>Not Found</title>

        <!-- Fonts -->
        <link rel="dns-prefetch" href="//fonts.gstatic.com">
        <link href="https://fonts.googleapis.com/css?family=Nunito" rel="stylesheet">

        <style>
            /*! normalize.css v8.0.1 | MIT License | github.com/necolas/normalize.css */html{line-height:1.15;-webkit-text-size-adjust:100%}body{margin:0}a{background-color:transparent}code{font-family:monospace,monospace;font-size:1em}[hidden]{display:none}html{font-family:system-ui,-apple-system,BlinkMacSystemFont,Segoe UI,Roboto,Helvetica Neue,Arial,Noto Sans,sans-serif,Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol,Noto Color Emoji;line-height:1.5}*,:after,:before{box-sizing:border-box;border:0 solid #e2e8f0}a{color:inherit;text-decoration:inherit}code{font-family:Menlo,Monaco,Consolas,Liberation Mono,Courier New,monospace}svg,video{display:block;vertical-align:middle}video{max-width:100%;height:auto}.bg-white{--bg-opacity:1;background-color:#fff;background-color:rgba(255,255,255,var(--bg-opacity))}.bg-gray-100{--bg-opacity:1;background-color:#f7fafc;background-color:rgba(247,250,252,var(--bg-opacity))}.border-gray-200{--border-opacity:1;border-color:#edf2f7;border-color:rgba(237,242,247,var(--border-opacity))}.border-gray-400{--border-opacity:1;border-color:#cbd5e0;border-color:rgba(203,213,224,var(--border-opacity))}.border-t{border-top-width:1px}.border-r{border-right-width:1px}.flex{display:flex}.grid{display:grid}.hidden{display:none}.items-center{align-items:center}.justify-center{justify-content:center}.font-semibold{font-weight:600}.h-5{height:1.25rem}.h-8{height:2rem}.h-16{height:4rem}.text-sm{font-size:.875rem}.text-lg{font-size:1.125rem}.leading-7{line-height:1.75rem}.mx-auto{margin-left:auto;margin-right:auto}.ml-1{margin-left:.25rem}.mt-2{margin-top:.5rem}.mr-2{margin-right:.5rem}.ml-2{margin-left:.5rem}.mt-4{margin-top:1rem}.ml-4{margin-left:1rem}.mt-8{margin-top:2rem}.ml-12{margin-left:3rem}.-mt-px{margin-top:-1px}.max-w-xl{max-width:36rem}.max-w-6xl{max-width:72rem}.min-h-screen{min-height:100vh}.overflow-hidden{overflow:hidden}.p-6{padding:1.5rem}.py-4{padding-top:1rem;padding-bottom:1rem}.px-4{padding-left:1rem;padding-right:1rem}.px-6{padding-left:1.5rem;padding-right:1.5rem}.pt-8{padding-top:2rem}.fixed{position:fixed}.relative{position:relative}.top-0{top:0}.right-0{right:0}.shadow{box-shadow:0 1px 3px 0 rgba(0,0,0,.1),0 1px 2px 0 rgba(0,0,0,.06)}.text-center{text-align:center}.text-gray-200{--text-opacity:1;color:#edf2f7;color:rgba(237,242,247,var(--text-opacity))}.text-gray-300{--text-opacity:1;color:#e2e8f0;color:rgba(226,232,240,var(--text-opacity))}.text-gray-400{--text-opacity:1;color:#cbd5e0;color:rgba(203,213,224,var(--text-opacity))}.text-gray-500{--text-opacity:1;color:#a0aec0;color:rgba(160,174,192,var(--text-opacity))}.text-gray-600{--text-opacity:1;color:#718096;color:rgba(113,128,150,var(--text-opacity))}.text-gray-700{--text-opacity:1;color:#4a5568;color:rgba(74,85,104,var(--text-opacity))}.text-gray-900{--text-opacity:1;color:#1a202c;color:rgba(26,32,44,var(--text-opacity))}.uppercase{text-transform:uppercase}.underline{text-decoration:underline}.antialiased{-webkit-font-smoothing:antialiased;-moz-osx-font-smoothing:grayscale}.tracking-wider{letter-spacing:.05em}.w-5{width:1.25rem}.w-8{width:2rem}.w-auto{width:auto}.grid-cols-1{grid-template-columns:repeat(1,minmax(0,1fr))}@-webkit-keyframes spin{0%{transform:rotate(0deg)}to{transform:rotate(1turn)}}@keyframes  spin{0%{transform:rotate(0deg)}to{transform:rotate(1turn)}}@-webkit-keyframes ping{0%{transform:scale(1);opacity:1}75%,to{transform:scale(2);opacity:0}}@keyframes  ping{0%{transform:scale(1);opacity:1}75%,to{transform:scale(2);opacity:0}}@-webkit-keyframes pulse{0%,to{opacity:1}50%{opacity:.5}}@keyframes  pulse{0%,to{opacity:1}50%{opacity:.5}}@-webkit-keyframes bounce{0%,to{transform:translateY(-25%);-webkit-animation-timing-function:cubic-bezier(.8,0,1,1);animation-timing-function:cubic-bezier(.8,0,1,1)}50%{transform:translateY(0);-webkit-animation-timing-function:cubic-bezier(0,0,.2,1);animation-timing-function:cubic-bezier(0,0,.2,1)}}@keyframes  bounce{0%,to{transform:translateY(-25%);-webkit-animation-timing-function:cubic-bezier(.8,0,1,1);animation-timing-function:cubic-bezier(.8,0,1,1)}50%{transform:translateY(0);-webkit-animation-timing-function:cubic-bezier(0,0,.2,1);animation-timing-function:cubic-bezier(0,0,.2,1)}}@media (min-width:640px){.sm\:rounded-lg{border-radius:.5rem}.sm\:block{display:block}.sm\:items-center{align-items:center}.sm\:justify-start{justify-content:flex-start}.sm\:justify-between{justify-content:space-between}.sm\:h-20{height:5rem}.sm\:ml-0{margin-left:0}.sm\:px-6{padding-left:1.5rem;padding-right:1.5rem}.sm\:pt-0{padding-top:0}.sm\:text-left{text-align:left}.sm\:text-right{text-align:right}}@media (min-width:768px){.md\:border-t-0{border-top-width:0}.md\:border-l{border-left-width:1px}.md\:grid-cols-2{grid-template-columns:repeat(2,minmax(0,1fr))}}@media (min-width:1024px){.lg\:px-8{padding-left:2rem;padding-right:2rem}}@media (prefers-color-scheme:dark){.dark\:bg-gray-800{--bg-opacity:1;background-color:#2d3748;background-color:rgba(45,55,72,var(--bg-opacity))}.dark\:bg-gray-900{--bg-opacity:1;background-color:#1a202c;background-color:rgba(26,32,44,var(--bg-opacity))}.dark\:border-gray-700{--border-opacity:1;border-color:#4a5568;border-color:rgba(74,85,104,var(--border-opacity))}.dark\:text-white{--text-opacity:1;color:#fff;color:rgba(255,255,255,var(--text-opacity))}.dark\:text-gray-400{--text-opacity:1;color:#cbd5e0;color:rgba(203,213,224,var(--text-opacity))}}
        </style>

        <style>
            body {
                font-family: 'Nunito';
            }
        </style>
    </head>
    <body class="antialiased">
        <div class="relative flex items-top justify-center min-h-screen bg-gray-100 dark:bg-gray-900 sm:items-center sm:pt-0">
            <div class="max-w-xl mx-auto sm:px-6 lg:px-8">
                <div class="flex items-center pt-8 sm:justify-start sm:pt-0">
                    <div class="px-4 text-lg text-gray-500 border-r border-gray-400 tracking-wider">
                        404                    </div>

                    <div class="ml-4 text-lg text-gray-500 uppercase tracking-wider">
                        Not Found                    </div>
                </div>
            </div>
        </div>
    </body>
</html>
* Closing connection 0

$ curl -v -X OPTIONS http://bar.localhost:8000/login
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to bar.localhost (127.0.0.1) port 8000 (#0)
> OPTIONS /login HTTP/1.1
> Host: bar.localhost:8000
> User-Agent: curl/7.64.1
> Accept: */*
>
< HTTP/1.1 200 OK
< Host: bar.localhost:8000
< Date: Fri, 25 Dec 2020 08:55:02 GMT
< Connection: close
< X-Powered-By: PHP/7.4.13
< Allow: GET,HEAD
< Cache-Control: no-cache, private
< Date: Fri, 25 Dec 2020 08:55:02 GMT
< Content-Type: text/html; charset=UTF-8
<
* Closing connection 0

@MilesChou MilesChou force-pushed the fix-options-method-bug branch from 20dd16e to c6ecece Compare December 25, 2020 14:59
@driesvints driesvints changed the title Fix OPTIONS method bug with use same path and diff domain when cache route [8.x] Fix OPTIONS method bug with use same path and diff domain when cache route Dec 28, 2020
driesvints
driesvints approved these changes Dec 28, 2020
View reviewed changes
Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

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

From a first look at things this lgtm.

@taylorotwell taylorotwell merged commit 75a0753 into laravel:8.x Dec 28, 2020
@MilesChou MilesChou deleted the fix-options-method-bug branch December 30, 2020 05:34
glamorous added a commit to glamorous/framework that referenced this pull request Apr 21, 2022
@glamorous
fix array keys from cached routes
36c59ff 
Introduced by laravel#35714.
Removed the slash to match the same array key as in
src/Illuminate/Routing/RouteCollection.php:60
@glamorous glamorous mentioned this pull request Apr 21, 2022
Merged
glamorous added a commit to glamorous/framework that referenced this pull request Apr 24, 2022
@glamorous
fix array keys from cached routes
5b98b72 
Introduced by laravel#35714.
Removed the slash to match the same array key as in
src/Illuminate/Routing/RouteCollection.php:60
taylorotwell pushed a commit that referenced this pull request Apr 25, 2022
@glamorous
fix array keys from cached routes (#42078)
d363435 
Introduced by #35714.
Removed the slash to match the same array key as in
src/Illuminate/Routing/RouteCollection.php:60
glamorous added a commit to glamorous/framework that referenced this pull request Apr 25, 2022
@glamorous
fix array keys from cached routes (laravel#42078)
e098411 
Introduced by laravel#35714.
Removed the slash to match the same array key as in
src/Illuminate/Routing/RouteCollection.php:60
taylorotwell pushed a commit that referenced this pull request Apr 25, 2022
@glamorous
fix array keys from cached routes (#42078) (#42124)
66970f7 
Introduced by #35714.
Removed the slash to match the same array key as in
src/Illuminate/Routing/RouteCollection.php:60
taylorotwell pushed a commit to illuminate/routing that referenced this pull request Apr 25, 2022
@glamorous
fix array keys from cached routes (#42078) (#42124)
a40b83b 
Introduced by laravel/framework#35714.
Removed the slash to match the same array key as in
src/Illuminate/Routing/RouteCollection.php:60
taylorotwell pushed a commit to illuminate/routing that referenced this pull request Apr 25, 2022
@glamorous
fix array keys from cached routes (#42078)
15b5248 
Introduced by laravel/framework#35714.
Removed the slash to match the same array key as in
src/Illuminate/Routing/RouteCollection.php:60
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@driesvints driesvints driesvints approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MilesChou @driesvints @taylorotwell