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

fix: Add file extention check into single middleware matcher. #3870

Merged
merged 2 commits into from
Jun 20, 2024

Conversation

bryan-robitaille
Copy link
Contributor

@bryan-robitaille bryan-robitaille commented Jun 20, 2024

Summary | Résumé

The current middleware was behaving unexpectedly with the current matcher array. The documentation was not clear that the array represents a logical OR between matchers and not a logical AND. The matchers were originally seperated for readability but have now been combined to ensure proper functionality.

Discovered because favicon.ico was having a language parameter added to it's path by the middleware which in turn caused the browser to receive a 404 when requesting the redirected path.

Screenshot 2024-06-20 at 12 18 19 PM
Screenshot 2024-06-20 at 12 18 05 PM

Test instructions | Instructions pour tester la modification

  • Open browser and navigate to the site.
  • Ensure favicon is displayed in the browser tab.

Pull Request Checklist

Please complete the following items in the checklist before you request a review:

  • Have you completely tested the functionality of change introduced in this PR? Is the PR solving the problem it's meant to solve within the scope of the related issue?
  • The PR does not introduce any new issues such as failed tests, console warnings or new bugs.
  • If this PR adds a package have you ensured its licensed correctly and does not add additional security issues?
  • Is the code clean, readable and maintainable? Is it easy to understand and comprehend.
  • Does your code have adequate comprehensible comments? Do new functions have docstrings?
  • Have you modified the change log and updated any relevant documentation?
  • Is there adequate test coverage? Both unit tests and end-to-end tests where applicable?
  • If your PR is touching any UI is it accessible? Have you tested it with a screen reader? Have you tested it with automated testing tools such as axe?

@bryan-robitaille bryan-robitaille added the bug Something isn't working label Jun 20, 2024
@bryan-robitaille bryan-robitaille marked this pull request as ready for review June 20, 2024 16:16
Copy link
Contributor

@bryan-robitaille bryan-robitaille changed the title Add file extention check into single middleware matcher. fix: Add file extention check into single middleware matcher. Jun 20, 2024
@bryan-robitaille bryan-robitaille enabled auto-merge (squash) June 20, 2024 16:24
Match all request paths except for the ones starting with:
- _next/static (static files)
- _next/image (image optimization files)
- img (public image files)
- static (public static files)
- react_devtools (React DevTools)
- contain files with extentions
Copy link
Contributor

Choose a reason for hiding this comment

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

small typo extension

@bryan-robitaille bryan-robitaille merged commit f4b92bc into develop Jun 20, 2024
13 checks passed
@bryan-robitaille bryan-robitaille deleted the fix/middleware branch June 20, 2024 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants