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

[8.x] Fix refresh during down #42217

Merged
merged 1 commit into from
May 2, 2022
Merged

[8.x] Fix refresh during down #42217

merged 1 commit into from
May 2, 2022

Conversation

driesvints
Copy link
Member

This PR fixes an issue where the Refresh header was not set when an application is down with a specific template. There was a missing header at the bottom of the maintenance file.

Fixes #42208

@driesvints driesvints changed the title Fix refresh during down [8.x] Fix refresh during down May 2, 2022
@X-Coder264
Copy link
Contributor

https://github.com/laravel/framework/blob/v9.10.1/src/Illuminate/Foundation/Http/Middleware/PreventRequestsDuringMaintenance.php#L74

Shouldn't the $this->getHeaders($data) call already add this header?

    /**
     * Get the headers that should be sent with the response.
     *
     * @param  array  $data
     * @return array
     */
    protected function getHeaders($data)
    {
        $headers = isset($data['retry']) ? ['Retry-After' => $data['retry']] : [];

        if (isset($data['refresh'])) {
            $headers['Refresh'] = $data['refresh'];
        }

        return $headers;
    }

@driesvints
Copy link
Member Author

driesvints commented May 2, 2022

@X-Coder264 maintenance mode only reaches that code if there's no custom template. If there is, the request bubbles down to the lines I added.

@X-Coder264
Copy link
Contributor

Are you saying that this block which is supposed to be reached when there is a custom template is never reached, thus being dead code?

            if (isset($data['template'])) {
                return response(
                    $data['template'],
                    $data['status'] ?? 503,
                    $this->getHeaders($data)
                );
            }

@driesvints
Copy link
Member Author

@X-Coder264 I think you misunderstand how this works. The maintenance.php file is checked first, see https://github.com/laravel/laravel/blob/9.x/public/index.php#L19. Only if all the conditions in that file fail, the request is propagated to the app, otherwise it bubbles down to the lines I changed.

@X-Coder264
Copy link
Contributor

Ah okay, yeah, I was missing that part, thanks for the explanation.

@taylorotwell taylorotwell merged commit 832d1df into 8.x May 2, 2022
@taylorotwell taylorotwell deleted the fix-refresh-during-down branch May 2, 2022 13:54
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.

php artisan down --refresh=5 does not work in combination with --render="maintenance.index"
3 participants