squidfunk / mkdocs-material

Documentation that simply works
https://squidfunk.github.io/mkdocs-material/
MIT License
20.3k stars 3.48k forks source link

Privacy plugin misbehave with query parameters in script-src #6977

Closed meysam81 closed 5 months ago

meysam81 commented 5 months ago

Context

Hello, and thanks (again) for the great project

I actually don't know if this a bug or an intended behavior, considering that I am by no means a frontender and can't center align a div :shrug:

I use GitHub pages to host my tech blog at https://developer-friendly.blog

Bug description

When I place the following extrahead section in my overrides/main.html...

{% block extrahead %}
  {{ super() }}
  <script async src="https://httpbin.org/redirect-to?url=https://httpbin.org/json" crossorigin="anonymous"></script>
{% endblock %}

... I get the following result when opening the View Page Source:

<script async= src="assets/external/httpbin.org/redirect-to.0cc65c20" crossorigin="anonymous">

The url query parameter is missing and for external services, that results to unintended behavior.

I gotta say, that the value of that query parameter is often found in the content of the downloaded "asset" when I less or cat it.

But either way, I can't be sure if this is the intended behavior of privacy plugin, or a limitation of it (which isn't documented BTW).

Related links

Reproduction

Try the overrides with the sample following url:

https://httpbin.org/redirect-to?url=https://httpbin.org/json

The resulting build will mess up with the url query parameter and won't pass it to the upstream server.

I wanted to use info plugin to attach my reproducible zip, but I got the following error, which beats the whole purpose of this issue:

INFO    -  Started archive creation for bug report
ERROR   -  Please remove 'custom_dir' setting.

  When reporting issues, you must remove all customizations
  and check if the problem persists. If not, the problem is
  caused by your overrides. Please understand that we can't
  help you debug your customizations. Please remove:

  - theme.custom_dir
  - hooks

  Additionally, please remove all third-party JavaScript or
  CSS not explicitly mentioned in our documentation:

  - extra_css
  - extra_javascript

  If you're using customizations from the theme's documentation
  and you want to report a bug specific to those customizations
  then set the 'archive_stop_on_violation: false' option in the
  info plugin config.

Steps to reproduce

  1. mkdocs new .
  2. Have the following minimal working example in your mkdocs.yml:
    site_name: My Docs
    theme:
    name: material
    custom_dir: overrides
    plugins:
    - privacy
  3. mkdir overrides && touch overrides/main.html
  4. Add the following content to your overrides/main.html
    
    {% extends "base.html" %}

{% block extrahead %} {{ super() }}

{% endblock %}

{% block announce %}{% include "announce.html" ignore missing %}{% endblock %}

{% block scripts %} {{ super() }} {% endblock %}


5. `mkdir build` or `mkdir serve` and open <http://localhost:8000> and open the View Page Source.
6. The query parameter of `url` is missing

### Browser

Firefox

### Before submitting

- [X] I have read and followed the [bug reporting guidelines](https://squidfunk.github.io/mkdocs-material/contributing/reporting-a-bug/).
- [X] I have attached links to [the documentation](https://squidfunk.github.io/mkdocs-material/), and possibly related [issues](https://github.com/squidfunk/mkdocs-material/issues) and [discussions](https://github.com/squidfunk/mkdocs-material/discussions).
- [X] I assure that I have [removed all customizations](https://squidfunk.github.io/mkdocs-material/contributing/reporting-a-bug/#remove-customizations) before submitting this bug report.
- [X] I have attached a __.zip file__ with a [minimal reproduction](https://squidfunk.github.io/mkdocs-material/guides/creating-a-reproduction/) using the [built-in info plugin](https://squidfunk.github.io/mkdocs-material/plugins/info/).

## More Context

For a better understanding of my use-case, I am trying to integrate Google Adsense as explained [here](https://support.google.com/adsense/answer/9274516).

But, even when the site is deployed, I still don't see the integration being successful (as seen below):

![image](https://github.com/squidfunk/mkdocs-material/assets/30233243/145bb832-3932-4c22-b4dc-0d1b1323e518)
alexvoss commented 5 months ago

The privacy plugin downloads assets and stores them locally so that the browser does not access any third-party web server when the user views the page(s). As a result, there is nothing that would interpret the query parameter since the assets are loaded as they are from the hosting web server. I would say that this is the plugin working as intended. That is assuming it has followed the redirect mentioned in your example and stored the file that it actually resolved to?

I am not sure that trying to use the privacy plugin with Google Adsense makes a lot of, um, sense? Are you aiming to reduce the footprint of your site with the exception of that service?

meysam81 commented 5 months ago

@alexvoss

I observed a great performance improvement when using privacy to download some of the external assets, especially on mobile devices (as measured by cloudflare and google pagespeed).

You're probably referring to the privacy exception, aren't you?

alexvoss commented 5 months ago

That is a side effect of the plugin's function - if the external hosting of the assets is slow. Normally, assets would sit on a CDN and that would improve the performance experienced by most users. For popular assets it would also help increase the chances that they are already cached by the browser. How many assets do you have where your own hosting is faster? It might make sense to manually integrate them into your site instead of asking the privacy plugin to do this?

I don't have much experience with the plugin and the exception clause. It does sound like what you would need to exclude Adsense code from being turned into local assets. Might be the way to go if there are too many assets to deal with manually - of if you want them to be downloaded afresh when you build.

Just looking at the options available, you can also combine the exclude statement with an include one if you want to use the plugin to only download assets from a specific site.

Do let us know if anything is not working. Note that the privacy plugin is marked as experimental.

squidfunk commented 5 months ago

The url query parameter is missing and for external services, that results to unintended behavior.

@alexvoss is right – the plugin is working as intended:

The privacy plugin downloads assets and stores them locally so that the browser does not access any third-party web server when the user views the page(s). As a result, there is nothing that would interpret the query parameter since the assets are loaded as they are from the hosting web server.

In order to create discernible URLs for different query parameters, the privacy plugin coerces them to a hash, since query parameters are not evaluated for self-hosted assets (an assumption by the plugin):

https://github.com/squidfunk/mkdocs-material/blob/545803977829e05fdc4f0c3b6c0e0cd9a72fde84/src/plugins/privacy/plugin.py#L501-L509

You can use assets_exclude to instruct the privacy plugin to not process the URL, in order to resolve this issue by a simple configuration change:

plugins:
  - privacy:
      assets_exclude: 
        - httpbin.org/*

Additionally, next time, please provide a self-contained minimal reproduction. It is mandatory. With a project of this size, we need to be efficient – we just cannot invest time to keep contents of files stored in different places around. Thank you!