Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Path table has src and dst limited to 128 chars, while HTTP specification has no limit over the length of a URL.
This patch does not make column size to be infinite, but increases it to 256, to accommodate longer URLs, while keeping the DB sane.
Comment | File | Size | Author |
---|---|---|---|
#50 | longer_paths-d6.diff | 3.94 KB | andrewsuth |
#45 | 288946-longer_paths-d6-45.diff | 3.91 KB | dawehner |
#42 | 288946-longer_paths-d6-42.diff | 3.2 KB | jeffschuler |
#34 | 288946-longer_paths_2-D6.diff | 3.66 KB | jeffschuler |
#30 | 288946-d6_longer_paths_2.diff | 3.66 KB | jeffschuler |
Comments
Comment #1
z.stolar CreditAttribution: z.stolar commentedI increased the column size to 256 instead of 255...
Rerolled the patch.
Comment #2
Anonymous (not verified) CreditAttribution: Anonymous commentedI agree that an arbitrary limit is wrong but 256 is still an arbitrary limit. Is there a reason to not make these a text column? Also, you need to supply an update for the existing tables. And what about the forms?
Comment #3
z.stolar CreditAttribution: z.stolar commentedI agree about the other fixes - I'll add them.
regarding the length - a question to those who know MySQL better: how to allow long input (>256chars) and still avoid new lines etc.? Should the validation be on Drupal side?
And what about HTML - can we have a text field longer than 256 chars?
Comment #4
Damien Tournoud CreditAttribution: Damien Tournoud commented@z.stolar: you can have several lines in a VARCHAR, too, there is no technical limitation.
About the limit, I would say that any website having paths longer than 128 characters would be seriously broken, and even more with 256 characters! I'm not sure this is even remotely needed.
Comment #5
z.stolar CreditAttribution: z.stolar commentedSo regarding new lines, and other characters which are not useful in a URL, we'll do validation during form validation.
@Damien: Why do you think the website may be broken? Even Explorer is capable of very long urls...
Comment #6
Damien Tournoud CreditAttribution: Damien Tournoud commented@z.stolar: of course, browsers are capable of handling long URLs, but that's really bad design.
Comment #7
z.stolar CreditAttribution: z.stolar commentedI think Drupal should assist people design their websites properly, but not take decisions for them.
I suggest we put some kind of recommendation to use not-so-long url aliases.
Can you please supply a short explanation of why it is considered bad design, so I'll add it to the path form?
Comment #8
Anonymous (not verified) CreditAttribution: Anonymous commentedWith pathauto, the URL's can be longer than the title of the node plus the length of the content type plus any length of text the user gives before that. I think 128 is just a shy short of good. I think any limit is bad. But we have to compromise in the middle I suppose; however, I don't think 256 is enough compromise and the columns should be set to text.
Comment #9
Damien Tournoud CreditAttribution: Damien Tournoud commented@earnie: well, there is no such thing as "no limit". Resources are *always* scarce. Pathauto should learn (if it doesn't already, I didn't verified) to guarantee that paths do not go over the limit.
I don't really care which limit it is (128 vs. 255), even if 128 looks *really* long already.
For reference, here is how such URL would look like:
This looks like a won't fix to me.
Comment #10
z.stolar CreditAttribution: z.stolar commentedI agree it *looks* bad.
But apart from that - why is it wrong?
Aesthetics is a matter of taste.
I think a good advice in the field's description is better than limiting the user. If someone decides to go with too long URLs, he should be noticed about it, not restricted from doing it.
Comment #11
z.stolar CreditAttribution: z.stolar commentedAttached patch modifies the forms (admin and node), and adds an update in system.install.
Comment #12
Anonymous (not verified) CreditAttribution: Anonymous commentedThe last submitted patch failed testing.
Comment #13
tootsietorres CreditAttribution: tootsietorres commentedMy 2 cents:
It is bad to limit the path if you are trying to send variables to a search. I have been building websites for Realtors and most use IDX to access the MLS on their websites. If they want a custom search link, 128 chars is really limiting.
Comment #14
jeffschulerRerolled to apply to current D7 HEAD.
Comment #15
smk-ka CreditAttribution: smk-ka commentedI was also running into this issue in the past when using pathauto, +1 for increasing the length to 255 characters.
@jeffschuler
ALTER TABLE is not the correct way to alter tables, use db_change_field() instead.
Comment #16
jeffschulerThanks smk-ka.
Re-rolled using db_change_field() in the db update function.
Comment #17
deekayen CreditAttribution: deekayen commentedFWIW, I don't see any more issues. Upgrade worked for me. Don't know if there should be a test for checking adding of 254, 255, and 256 char strings to make sure 256 fails, etc.
Anyone else in favor of splitting off to a separate issue to discuss why url_alias is managed in system instead of path?
Comment #18
Dries CreditAttribution: Dries commentedWaw, do we really need that long URLs?
Comment #19
deekayen CreditAttribution: deekayen commentedI think the answer to that question is another question. Why 128 then? Why not 100 or 200? Those are at least round numbers.
255 at least maxes out a field limit at the db level and gives users more flexibility to use the site as they wish.
Comment #20
sunWe are limiting something that should not have a limit.
Just because Drupal core does not exceed the limit does not mean that there is not a use-case that may be limited by the fake limit we apply.
Comment #21
sunThat said, please remove that second, blank PHPDoc line:
Comment #22
Dries CreditAttribution: Dries commentedFair enough, even though it feels a bit silly. Committed to CVS HEAD.
Comment #24
will_in_wi CreditAttribution: will_in_wi commentedCan this be backported to 6.x? I have a use case where I have an external bound link which is 226 chars long. I have a hack in place at the moment which makes it work, but it would be nice to fix this for the current version.
Comment #25
Dave ReidComment #26
febbraro CreditAttribution: febbraro commentedHere is a patch for D6 that we have in place and working. Thanks to @emackn for making it a while ago.
Comment #27
jeffschulerAwesome, thanks...
Setting to needs review.
Comment #28
HnLn CreditAttribution: HnLn commentedsubscribe and I don't think it's silly :-)
use case: pathauto aliasses based on menu-path (e.g. a product catalogue, I want the path to reflect the category tree the product belongs to)
Comment #30
jeffschulerre-rolled for current Drupal 6.x-dev
Comment #32
spamwelle CreditAttribution: spamwelle commentedhow could i change it to unlimited length and is there a fix for drupal 6.19?
Comment #33
Eric_A CreditAttribution: Eric_A commentedThis D6 patch will go from "needs review" to "needs work" automatically if the patch file is not properly named. That is because all test cases run by the bot are for Drupal 7.
From the File attachments fieldset:
For patches that apply specifically to Drupal 5 or Drupal 6, you can prefix the extension with -D5 or -D6 to prevent them from being queued for testing.
Comment #34
jeffschulerThanks @Eric_A.
Patch from #30 still applies. I've renamed it.
Comment #35
amfis CreditAttribution: amfis commentedWhy this is not in the Drupal core yet?
Comment #36
Eric_A CreditAttribution: Eric_A commentedIt is in core, just not in D6. It will only be considered after the community has reviewed the code, tested the patch and changed the status accordingly.
Personally at this stage of the D6 life cycle I feel uncomfortable with code changes that require some module maintainers to think and think again when writing new code because they may need to implement hook_requirements() to enforce Drupal 6.20 minimum or provide code for both scenario's. The fact of the matter is that new code just cannot rely on the form field and db field to be 255 chars long.
Comment #37
rsanuy CreditAttribution: rsanuy commented#14: longer_paths_2.patch queued for re-testing.
Comment #38
jfarry CreditAttribution: jfarry commented+1 subscribed.
This becomes a huge issue when linking to files from the menu system, on multisite installs.
I don't know if there is a way around this, but this is the problem that I have:
http://multisitedubdomain.intranet.multisiteurl.com/sites/multisitesubdo...
I can't add this in as a relative path either :( If anyone has another solution, I'm all ears :)
Comment #39
Anonymous (not verified) CreditAttribution: Anonymous commentedMove to D8 since this is a feature request we need to apply there first.
Comment #40
jfarry CreditAttribution: jfarry commentedI understand your reasoning and i mean no disrespect, but this has been effecting d6 users for the last three years, whereas nobody is using drupal 8 yet and a patch has already been implemented in D7 core.
I am still learning about the patching/git/how the setup works and I'll have a go at it when i understand all this information, but in the interim, I (and probably about 15 other users from this thread) would not complain at all if this was fixed before I can get to it.
Cheers :)
Comment #41
Eric_A CreditAttribution: Eric_A commentedYes, this is in D8 and D7, so back to D6. The bot needs a -p1 patch. The -p0 format has been phased out.
Comment #42
jeffschulerRe-rolled against drupal-6.x-dev (with git diff).
Comment #43
jfarry CreditAttribution: jfarry commentedI've manually applied this to my D6 6.22 install, then run update.php, run cron, done a full cache flush and when creating a new menu item or editing an old one, the max field size (for the edit-menu-link-path field) is still 128.
I've triple checked my implementation, but it's exactly as described in the diff. Is there something else that I could be doing wrong?
Comment #44
colanpyrhoe: There's a separate issue for menu items. Take a look at #1237290: Menu path length is limited by the UI to 128 characters.
Comment #45
dawehnerI personally think this two patches should be merged as it's a small change and saves some time of the process (review testing etc.)
Comment #46
hillaryneaf CreditAttribution: hillaryneaf commentedTested D6 patch, works for me. Thanks!
Comment #47
andrewsuth CreditAttribution: andrewsuth commentedThis has not yet been pushed into Drupal 6 core - any idea of when it will be done?
I feel this is really important as it is causing duplicate alias' to be created.
In path.module, this logic is performed:
$existing = db_fetch_array(db_query("SELECT pid, src FROM {url_alias} WHERE dst = '%s' AND language = '%s' ORDER BY pid DESC", $alias, $language));
So when the alias is > 128 characters, it does not find the entry in the database (as they are truncated to 128 characters at the DB field level) so it continues to reinserts the alias entry, causing duplicates.
Comment #48
colanThis will not be committed until the status is RTBC. You can set that yourself if you've tested the latest patch and it looks good + works.
Comment #49
j0rd CreditAttribution: j0rd commentedJust ran into this problem on a D6 site. I simply added #maxlength to link_path to get me passed the UI defaulting to 128 maxlength.
I've applied patch #45 and it resolved my problem.
From a review of the code it seems like sane changes (looks like he got most of the places to change), and the database update worked as expected.
Would like to see this committed.
Comment #50
andrewsuth CreditAttribution: andrewsuth commentedPatch #45 is not OK as it sets the default column value to
NULL
rather than empty string.In the system_scheme we see that the default is set to empty string:
But then in the database update, the default value is not set, which causes it to reset to NULL:
The correct db_change_field() to execute should be:
Comment #51
colanComment #52
j0rd CreditAttribution: j0rd commentedGood catch @andrewsuth.
Comment #53
Viliasas CreditAttribution: Viliasas commentedAre there any news about this change? Will it be pushed to Drupal core at all?