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

com_redirect adding urls larger than 255 #4781

Closed

Conversation

Kixo
Copy link
Contributor

@Kixo Kixo commented Oct 17, 2014

.after some reading up I found up that maximum allowed URL in IE is 2083, other browsers allow much higher numbers, even over 100.000, but we must stand with the lowest one, so I set it up for 2083.
Example description here: http://www.boutell.com/newfaq/misc/urllength.html

In this PR I have removed unique constraint. When we insert new row we are actually doing this check inside of the Joomla code, so this is not needed. And it was making a trouble because maximum varchar key for unique key is 767. So while removing it for mysql, I removed it for all DBs because as stated above, it is not needed.

Please check this code against Postgre and sqlazure if possible.

@Kixo
Copy link
Contributor Author

Kixo commented Oct 17, 2014

ok

after some reading up I found up that maximum allowed URL in IE is 2083, other browsers allow much higher numbers, even over 100.000, but we must stand with the lowest one, so I set it up for 2083.
Example description here: http://www.boutell.com/newfaq/misc/urllength.html

In this PR I have removed unique constraint. When we insert new row we are actually doing this check inside of the Joomla code, so this is not needed. And it was making a trouble because maximum varchar key for unique key is 767. So while removing it for mysql, I removed it for all DBs because as stated above, it is not needed.

Please check this code against Postgre and sqlazure if possible.

http://issues.joomla.org/tracker/joomla-cms/4692

@jissues-bot jissues-bot changed the title 4692_com_redirect_adding_urls_larger_than_255 4692 com_redirect adding urls larger than 255 Oct 17, 2014
@jissues-bot jissues-bot changed the title 4692 com_redirect adding urls larger than 255 com_redirect adding urls larger than 255 Oct 17, 2014
@RCheesley
Copy link

Successful test, for reference test instructions are:

  1. Check _redirect_links structure view to see the fields old_url, new_url and referer are all of type varchar(255).
  2. Apply patch
  3. Go to Extensions Manager>Database and apply 'fix' (note I had one left over where it was trying to change back to 255 when on 3.3.7-dev, may be a quirk)
  4. All fields old_url, new_url and referer should now be varchar(2083)

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

@anibalsanchez
Copy link
Contributor

@test OK, following @RCheesley steps, structure modified successfully.

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

@brianteeman
Copy link
Contributor

Two good tests thanks setting RTC

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

@phproberto
Copy link
Contributor

This cannot be merged anymore. Also maybe we need to do it against v3.4-dev branch renaming sql files properly

@brianteeman
Copy link
Contributor

@Kixo please can you update your PR as it can no longer be merged. Thanks

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

…692_com_redirect_adding_urls_larger_than_255

Conflicts:
	installation/sql/mysql/joomla.sql
	installation/sql/postgresql/joomla.sql
	installation/sql/sqlazure/joomla.sql
@Kixo
Copy link
Contributor Author

Kixo commented Nov 28, 2014

@brianteeman updated

…692_com_redirect_adding_urls_larger_than_255

Conflicts:
	installation/sql/mysql/joomla.sql
	installation/sql/postgresql/joomla.sql
	installation/sql/sqlazure/joomla.sql
@brianteeman brianteeman removed the PBF14 label Jan 1, 2015
@waader
Copy link
Contributor

waader commented Jan 9, 2015

@test failed for mssql

Please add the closing ) in installation/sql/sqlazure/joomla.sql line 2364.

Unrelated to that but found in the course of testing: Saving a link in the redirect component gives an error when using postgresql or mssql as database because NULL cannot be inserted into column "referer".

@Kixo
Copy link
Contributor Author

Kixo commented Jan 26, 2015

thanks for testing @waader I uninstalled mssql so I did not test changes for the second time.
I have updated the PR with corrected SQL.

@waader
Copy link
Contributor

waader commented Jan 27, 2015

There is another small problem. In 3.4.0-2014-09-16.sql

ALTER TABLE #__redirect_links MODIFY new_url varchar(255);

results in this problem - when you update the database schema via "Extension manager" > Database > Fix:

Table 'uy937_redirect_links' does not have column 'new_url' with type 'varchar(255)'. (From file 3.4.0-2014-09-16.sql.)

So this line could be deleted.

@mbabker
Copy link
Contributor

mbabker commented Jan 27, 2015

That column is supposed to be in the table (see https://github.com/joomla/joomla-cms/blob/staging/installation/sql/mysql/joomla.sql#L1517).

@waader
Copy link
Contributor

waader commented Jan 27, 2015

Of course, but there is a conflict with this statement in 3.4.0-2014-10-17.sql

ALTER TABLE #__redirect_links CHANGE new_url new_url varchar(2083) NOT NULL;

…692_com_redirect_adding_urls_larger_than_255
@Kixo
Copy link
Contributor Author

Kixo commented Mar 6, 2015

@waader I am not exactly sure what the problem is. Can you please explain in more detail or propose the solution.

@waader
Copy link
Contributor

waader commented Mar 6, 2015

The proposed solution was to delete

ALTER TABLE #__redirect_links MODIFY new_url varchar(255);

in 3.4.0-2014-09-16.sql.

@Kixo
Copy link
Contributor Author

Kixo commented Mar 6, 2015

ok updated

@waader
Copy link
Contributor

waader commented Mar 6, 2015

@test works with mysql and will probably work with postgresql and mssql. But there is another unrelated problem there. Thanks Kixo!

@brianteeman
Copy link
Contributor

Please see #9269

@brianteeman brianteeman closed this Mar 1, 2016
wilsonge added a commit that referenced this pull request Mar 1, 2016
Allow URLs with lengths up to 2048 chars in com_redirect, redo of PR #4781
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

9 participants