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

Projects
None yet
9 participants
@Kixo
Copy link
Contributor

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

This comment has been minimized.

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

This comment has been minimized.

Copy link

RCheesley commented Oct 17, 2014

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.

@nicksavov nicksavov added the PBF14 label Oct 17, 2014

@anibalsanchez

This comment has been minimized.

Copy link
Contributor

anibalsanchez commented Oct 17, 2014

@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

This comment has been minimized.

Copy link
Contributor

brianteeman commented Oct 17, 2014

Two good tests thanks setting RTC

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

@phproberto

This comment has been minimized.

Copy link
Contributor

phproberto commented Oct 24, 2014

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

@brianteeman

This comment has been minimized.

Copy link
Contributor

brianteeman commented Nov 27, 2014

@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.

Merge branch 'staging' of https://github.com/joomla/joomla-cms into 4…
…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

This comment has been minimized.

Copy link
Contributor Author

Kixo commented Nov 28, 2014

@brianteeman updated

Merge branch 'staging' of https://github.com/joomla/joomla-cms into 4…
…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

This comment has been minimized.

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 added some commits Jan 26, 2015

@Kixo

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

mbabker commented Jan 27, 2015

@waader

This comment has been minimized.

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;

Merge branch 'staging' of https://github.com/joomla/joomla-cms into 4…
…692_com_redirect_adding_urls_larger_than_255
@Kixo

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

Kixo commented Mar 6, 2015

ok updated

@waader

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

brianteeman commented Mar 1, 2016

Please see #9269

@brianteeman brianteeman closed this Mar 1, 2016

wilsonge added a commit that referenced this pull request Mar 1, 2016

Merge pull request #9269 from richard67/long-urls-in-redirect-component
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
You can’t perform that action at this time.