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

Don't append but set the "Content-Encoding" header value in htaccess.txt #39205

Conversation

richard67
Copy link
Member

@richard67 richard67 commented Nov 12, 2022

Pull Request for Issue #39176 .

Summary of Changes

The Header append Content-Encoding gzip is changed to Header set Content-Encoding gzip in the section for the gzipped resource files of the htaccess.txt file.

This makes sure not to get an invalid value "gzip gzip" value but a correct "gzip" for the "Content-Encoding" header of the gzipped resource files when following conditions are met:

  • Apache web server with mod_rewrite in use
  • mod_gzip or mod_deflate are not enabled (otherwise you get double compression anyway).
  • Joomla is installed in a subdirectory of your domain or your local webserver's document root.
  • An .htaccess file is used in that subdirectory which is based on the Joomla 4 htaccess.txt file and so has the last section for the gzipped resource files in use.
  • The gzipped resource files are present, i.e. you are not on a Git clone but on a real Joomla installation made with an installation package (on a Git clone you can run npm run gzip to create the gzipped resources for testing).
  • In the parent directory, i.e. the domain directory or your local webserver's document root is also an .htaccess file like that in use. That can be the case e.g. if you have the above mentioned Joomla installation in a subdirectory of a Joomla installation.

See also the Drupal issue https://www.drupal.org/project/drupal/issues/1116416 linked in #39176 for a detailed description and discussion of the issue.

As concluded there in the discussion, the initial approach to do the same change also for the later Header append Vary Accept-Encoding line was abandoned because that header value can be a list of multiple http headers, see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Vary . Therefore I also don't change that in our htaccess.txt.

In addition, a postinstall message is added, but only for updated sites, not for new installations, as the latter come already with the correct htaccess.txt file.

Testing Instructions

1. Testing the .htaccess change

  1. Have an Apache webserver with mod_rewrite enabled and mod_gzip and mod_deflate disabled.

  2. Have an .htaccess file in your webserver's document root which contains at least the last section of our Joomla 4 htaccess.txt file:

<IfModule mod_headers.c>
	# Serve gzip compressed CSS files if they exist
	# and the client accepts gzip.
	RewriteCond "%{HTTP:Accept-encoding}" "gzip"
	RewriteCond "%{REQUEST_FILENAME}\.gz" -s
	RewriteRule "^(.*)\.css" "$1\.css\.gz" [QSA]

	# Serve gzip compressed JS files if they exist
	# and the client accepts gzip.
	RewriteCond "%{HTTP:Accept-encoding}" "gzip"
	RewriteCond "%{REQUEST_FILENAME}\.gz" -s
	RewriteRule "^(.*)\.js" "$1\.js\.gz" [QSA]

	# Serve correct content types, and prevent mod_deflate double gzip.
	RewriteRule "\.css\.gz$" "-" [T=text/css,E=no-gzip:1]
	RewriteRule "\.js\.gz$" "-" [T=text/javascript,E=no-gzip:1]

	<FilesMatch "(\.js\.gz|\.css\.gz)$">
		# Serve correct encoding type.
		Header append Content-Encoding gzip

		# Force proxies to cache gzipped &
		# non-gzipped css/js files separately.
		Header append Vary Accept-Encoding
	</FilesMatch>
</IfModule>

This can be because you already have a Joomla 4 installation there and you use URL rewriting.

  1. Create a new subdirectory below that and install a Joomla 4.2.5 or latest 4.2.6-dev nightly build into that directory.

  2. Enable URL rewriting for that installation and copy htaccess.txt to .htaccess.

  3. Request some page of that installation with a forced reload and watch the network tab of your browser's developer tools.

  4. Check the "Content-Encoding" in the http response of one of the gzipped css or js resources.
    Result: Value "gzip gzip", see section "Actual result BEFORE applying this Pull Request" below.

  5. Change the .htaccess file of the installation in the subdirectory in the same way as this PR changes the htaccess.txt file.

  6. Repeat steps 4 and 5.
    Result: Value "gzip", see section "Expected result AFTER applying this Pull Request" below.

2. Testing the postinstall message

  1. Make a new installation with the installation package created by drone for this branch and verify that the new postinstall message is not shown.

  2. Update a 4.2.8 or older or a current 4.2-dev branch to the update package created by drone for this PR and verify that after the update the new postinstall message is shown.

Actual result BEFORE applying this Pull Request

Header "Content-Encoding" has value "gzip gzip" in the http response for a gzipped resource file.

Expected result AFTER applying this Pull Request

Header "Content-Encoding" has value "gzip" in the http response for a gzipped resource file.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org: https://docs.joomla.org/Preconfigured_htaccess

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

When the PR gets merged, I will care for the update of the documentation mentioned above.

@brianteeman
Copy link
Contributor

But it maybe could make sense to mention the change in the release announcement or an FAQ document for that release when this PR will be released so people change their existing .htaccess file.

In the past changes in the htaccess.txt file have been accompanied by a post-installation message.

@vicn1222
Copy link

I have tested this item ✅ successfully on 60109ea

After changing from
Header append Content-Encoding gzip

to
Header set Content-Encoding gzip

The joomla works properly in both standalone installation and in a subdirectory where gzip has already been applied. Without the change, joomla installed in subdirectory causes error due to double encoding of gzip.

Thank you Rchard!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39205.

@richard67
Copy link
Member Author

But it maybe could make sense to mention the change in the release announcement or an FAQ document for that release when this PR will be released so people change their existing .htaccess file.

In the past changes in the htaccess.txt file have been accompanied by a post-installation message.

Yes, I remember now. Will do that when there is feedback telling that it should be done.

@sandewt
Copy link
Contributor

sandewt commented Nov 13, 2022

No documentation changes for...

I think a change is needed here:

https://docs.joomla.org/Preconfigured_htaccess


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39205.

@richard67
Copy link
Member Author

richard67 commented Nov 13, 2022

No documentation changes for...

I think a change is needed here:

https://docs.joomla.org/Preconfigured_htaccess

To me that seems to be outdated document which does not describe our standard htaccess.txt as it is now. I am reluctant to do anything with that and will close this PR if I am forced to update that.

@sandewt
Copy link
Contributor

sandewt commented Nov 13, 2022

To me that seems to be outdated document which does not describe our standard htaccess.txt as it is now.

To be honest, I didn't look for which joomla version this .httaccess is for.

I am reluctant to do anything with that and will close this PR if I am forced to update that.

I understand. Don't close, because this PR is of great use and importance.

@richard67
Copy link
Member Author

To me that seems to be outdated document which does not describe our standard htaccess.txt as it is now.

To be honest, I didn't look for which joomla version this .httaccess is for.

I am reluctant to do anything with that and will close this PR if I am forced to update that.

I understand. Don't close, because this PR is of great use and importance.

Hmm, I've just checked again, this time on my desktop with a bigger screen, and it seems I was wrong and that documentation is still in use and should be updated when this PR here gets merged.

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 60109ea

👍


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39205.

@Quy
Copy link
Contributor

Quy commented Feb 14, 2023

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/39205.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 14, 2023
@Hackwar Hackwar added the Small A PR which only has a small change label Feb 25, 2023
@brianteeman
Copy link
Contributor

This really should get a post installation message before it can be merged

@richard67
Copy link
Member Author

This really should get a post installation message before it can be merged

@brianteeman I think it should only be shown for updates, right? Or also for new installs? Could you point me to an other postinstall message which I could use as example?

@brianteeman
Copy link
Contributor

Just for updates will be fine. There are several examples in the administrator/components/com_admin/postinstall folder

@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Mar 7, 2023
@richard67
Copy link
Member Author

I have added a postinstall message for updated sites only and updated the testing instructions. It needs now human tests for the 2nd test for the postinstall message.

It also needs to update the example given in the documentation here https://docs.joomla.org/Preconfigured_htaccess . But I think this should be done after this PR has been merged so that documentation does not tell something which is not implemented yet.

@richard67
Copy link
Member Author

I have to fix a syntax error in the update SQL for PostgreSQL. Stay tuned.

@richard67
Copy link
Member Author

Done. Works.

@fancyFranci
Copy link
Contributor

I tested the package and the new post installation message appears as expected. Thanks for adding it to the PR and of course for the fix itself!

@fancyFranci fancyFranci merged commit 1b763de into joomla:4.2-dev Mar 7, 2023
@fancyFranci fancyFranci added this to the Joomla! 4.2.9 milestone Mar 7, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 7, 2023
@richard67
Copy link
Member Author

Thanks

@richard67 richard67 deleted the 4.2-dev-htaccess-txt-duplicate-content-encoding branch March 7, 2023 20:42
@richard67
Copy link
Member Author

I've just updated the content of the .htaccess file on https://docs.joomla.org/Preconfigured_htaccess with the change from this PR.

That docs page shows at the top a hint telling that there are changes which have not been marked for translation.

This was already the case before my edit.

@toivo Do you have an idea how to fix that?

@toivo
Copy link
Contributor

toivo commented Mar 7, 2023

@richard67 Nothing in the last 30 days was listed, then I changed 30 to 300 in the address bar and the following sentence was listed:

Enter a page name to see changes on pages linked to or from that page. (To see members of a category, enter Category:Name of category). Changes to pages on your Watchlist are in bold.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Required Language Change This is for Translators Small A PR which only has a small change
Projects
None yet
Development

Successfully merging this pull request may close these issues.