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

Added shortcuts via RewriteRule to .htaccess #1272

Merged
merged 1 commit into from
Apr 13, 2022
Merged

Conversation

nagmat84
Copy link
Collaborator

While I was looking for a solution to #1217 (comment), I discovered that our .htaccess could be improved.

As there is a lot (I mean really A LOT) requests for the upload directory, we can take a shortcut there. The new rules also avoid some unnecessary exceptions and logging, if a browser requests a non-existing file.

@codecov
Copy link

codecov bot commented Apr 12, 2022

Codecov Report

Merging #1272 (af88ee1) into master (760839b) will decrease coverage by 0.96%.
The diff coverage is n/a.

@nagmat84 nagmat84 requested review from kamil4 and ildyria April 12, 2022 19:20
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@nagmat84
Copy link
Collaborator Author

nagmat84 commented Apr 12, 2022

While I was working on the .htaccess file, I also wondered if these lines

Lychee/public/.htaccess

Lines 12 to 15 in 4d1b5f0

# Redirect Trailing Slashes If Not A Folder...
RewriteCond %{REQUEST_FILENAME} !-d
RewriteCond %{REQUEST_URI} (.+)/$
RewriteRule ^ %1 [L,R=301]
are actually correct. Here, %1 in the RewriteRule captures everything of the URL but the trailing /, then the RewriteRule replaces the beginning of the URL ^ by %1. The value of %1 is put it front of the URL which is already there, i.e. if my-url/ is not an existing directory, then the browser is redirected to the URL my-urlmy-url/. I don't think this is intended, and the rule should be RewriteRule .* %1 [L,R=301].

Why do we need the rules at all? What purpose do they serve? @ildyria added them to .htaccess.

@nagmat84 nagmat84 changed the title Added rewrite shortcuts to .htaccess Added shortcuts via RewriteRule to .htaccess Apr 12, 2022
@ildyria
Copy link
Member

ildyria commented Apr 13, 2022

I never touched that part of the file.
C.f. original here: https://github.com/laravel/laravel/blob/9.x/public/.htaccess

Copy link
Member

@ildyria ildyria left a comment

Choose a reason for hiding this comment

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

Tested. LGTM.

@ildyria ildyria merged commit 0199212 into master Apr 13, 2022
@ildyria ildyria deleted the improve_htaccess branch April 13, 2022 14: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.

2 participants