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.

Sign up for GitHub

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

Jump to bottom

CDN isnot applied on images with relative url containing query string #3724

Open
2 of 6 tasks
Mai-Saad opened this issue Mar 26, 2021 · 2 comments · May be fixed by #6195
Open
2 of 6 tasks

CDN isnot applied on images with relative url containing query string #3724

Mai-Saad opened this issue Mar 26, 2021 · 2 comments · May be fixed by #6195
Labels
effort: [XS] < 1 day of estimated development time module: CDN priority: medium Issues which are important, but no one will go out of business. severity: moderate Feature isn't working as expected but has work around to get same value type: bug Indicates an unexpected problem or unintended behavior

Comments

@Mai-Saad
Copy link

Mai-Saad commented Mar 26, 2021 •
edited

Before submitting an issue please check that you’ve completed the following steps:

  • Made sure you’re on the latest version

  • Used the search feature to ensure that the bug hasn’t been reported before

Describe the bug
A clear and concise description of what the bug is.
CDN is not applied on paths containing query string

To Reproduce
Precondition:

  • Having page with image URL containing query string i.e <img src="/wp-content/rocket-test-data/images/fixtheissue.jpg?123" >
    Steps to reproduce the behavior:
  1. In WPR settings -> CDN , enable CDN and add CDName i.e test.com
  2. open the page incognito and check source
  3. image URL didn't use CNAME

Expected behavior
CDN is applied and URL changes to src="https://test.com/wp-content/rocket-test-data/images/fixtheissue.jpg?123"

Screenshots
If applicable, add screenshots to help explain your problem.
Screenshot from 2021-03-26 19-01-17

Additional context
If the image URL is absolute URL with query string i.e <img src="http://new.rocketlabsqa.ovh/wp-content/rocket-test-data/images/fixtheissue.jpg?1234" > , CDN will be applied normally

Backlog Grooming (for WP Media dev team use only)

  • Reproduce the problem
  • Identify the root cause
  • Scope a solution
  • Estimate the effort
@Mai-Saad Mai-Saad added type: bug Indicates an unexpected problem or unintended behavior module: CDN priority: low Issues that can wait severity: moderate Feature isn't working as expected but has work around to get same value labels Mar 26, 2021
@GeekPress GeekPress added needs: grooming priority: medium Issues which are important, but no one will go out of business. and removed priority: low Issues that can wait labels Mar 30, 2021
@Mai-Saad
Copy link
Author

Note: CDN not applied on js relative URL too (probably any relative internal URL)
i.e <script src="/wp-content/rocket-test-data/scripts/testdummy2.js"> crossorigin='anonymous'</script>

@mostafa-hisham
Copy link
Contributor

mostafa-hisham commented Aug 25, 2022 •
edited

Identify the root cause ✅

tested it on images locally
<img src="/wp-content/download.jpeg" width="100" height="100">
worked fine

did not work on
<img src="../wp-content/download.jpeg" width="100" height="100">

Scope a solution ✅

To fix relative URLs starting with .
we need to change the regex in

wp-rocket/inc/Engine/CDN/CDN.php

Line 69 in bb4ab38

$relative_path_pattern = '|\/[^/](?:[^"\')\s>]+\.[[:alnum:]]+)';

to
|(\.+\/|\/)[^/](?:[^"')\s>]+\.[[:alnum:]]+) (worked with maybe there is better one)

Estimate the effort ✅

[xs]

@mostafa-hisham mostafa-hisham added effort: [XS] < 1 day of estimated development time and removed needs: grooming labels Aug 25, 2022
@mostafa-hisham mostafa-hisham linked a pull request Sep 28, 2023 that will close this issue
Open
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
effort: [XS] < 1 day of estimated development time module: CDN priority: medium Issues which are important, but no one will go out of business. severity: moderate Feature isn't working as expected but has work around to get same value type: bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix relative urls regex in cdn to work with dots wp-media/wp-rocket
3 participants
@GeekPress @mostafa-hisham @Mai-Saad