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

Missing "header" field added in database query #10214

Merged
merged 1 commit into from
May 7, 2016

Conversation

jo-sf
Copy link
Contributor

@jo-sf jo-sf commented May 3, 2016

If a full URL is redirected the fields selected from the database are "new_url", "header" and "published" but if a server-relative URL is redirected only the fields "new_url" and "published" are selected from the database.

This has two implications:

  • you will find an error message in your webserver error log file whenever a server-relative URL is redirected, this error message looks like: PHP Notice: Undefined property: stdClass::$header in .../plugins/system/redirect/redirect.php on line 142
  • if you enable the advanced feature for setting redirection codes and if you use this feature with server-relative redirections it is not working since the database field "header" holding this redirection code isn't selected from the database

This pull request changes the select with the server-relative URL such that in this case the "header" field is selected too.

How to test:

  • define a server-relative redirection and set the redirection code to 307
  • open the redirected URL either with your browser or with e.g. wget (for wget see below)
  • check for the redirection and the redirection code - the code is 301

Now apply the change and repeat the test, the redirection code is now 307.

Since browsers tend to cache redirections they got from a web server it is not easy to test the redirection plugin. One way is to completely clean the browser cache prior to the next test (or to remove at least all objects cached from the server used for testing).

The other way is to use the tool wget as follows:

wget -O - --max-redirect 0 <URL>

For please substitute the redirected URL. The relevant output of this tool will look like:

HTTP request sent, awaiting response... 307 Temporary Redirect
Location: <new URL>

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented May 3, 2016

I have tested this item ✅ successfully on 4ae6da2

I confirm the issue and this PR solves it.

How did i test.

  • Used latest staging
  • Went to Components -> Redirects (enabled the plugin)
  • Setted Advanced mode to Yes in Component Options
  • Created new redirect /en/testredirect1 to /en/testredirect2 with 307 HTTP status code
  • Used a private browser tab and disabled cache in chrome dev tools
  • Opened dev console in browser to check the "Network" tab
  • Introduced http://mydomain.tld/en/testredirect1 in browser address
  • Redirected to http://mydomain.tld/en/testredirect2
  • Checked in dev tools the HTTP status code of the redirect was a 301 (not a 307 as it should)
  • Checked server error log files and the notice described is there
  • Applied patch - tried again all went as it should: 307 redirect and no php notice

Thanks!


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

@zero-24
Copy link
Contributor

zero-24 commented May 7, 2016

I have tested this item ✅ successfully on 4ae6da2

Thanks. RTC


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

@zero-24
Copy link
Contributor

zero-24 commented May 7, 2016

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 7, 2016
@brianteeman brianteeman added this to the Joomla 3.6.0 milestone May 7, 2016
@wilsonge wilsonge merged commit 9e02024 into joomla:staging May 7, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 7, 2016
@wilsonge wilsonge added the RTC This Pull Request is Ready To Commit label May 7, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 7, 2016
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.

None yet

6 participants