Hi all,
We just updated to the latest version of Link and noticed URLs were getting truncated when displaying the node. Here's what the var dump is showing. The original_url below is what is entered in the field for that record.

["url"]=> string(47) "https://thriva.activenetwork.com/Reg4/Form.aspx"

["original_url"]=> string(79) "https://thriva.activenetwork.com/Reg4/Form.aspx?IDTD=8607700&RF=11910705..."

We had always used ['url'] in our node template.

Do you know what might be causing this? Again it just started after we updated the module a few days ago.

Thank you,

Terry

Comments

tcnolan7 created an issue. See original summary.

damienmckenna’s picture

What output settings are you using for that field?

tcnolan7’s picture

Hi Damien,

It's set to "URL, as plain text"

Terry

ron collins’s picture

Seems like we might be having the same problem. We hide the link in a view (the link field has the formatter "Title, as link (default)") and then we rewrite the title field with the token [field_link].

Every URL with a query param is cut off. Downgrading to 1.6 fixes.

I won't change it but I'd suggest this is higher than normal priority and probably a quick fix.

sashken2’s picture

I have same problem too.

I use "URL with the protocol removed" formatter.

damienmckenna’s picture

Version: 7.x-1.7 » 7.x-1.x-dev
Category: Support request » Bug report
Issue tags: +Needs tests

Seems like we need to improve the test coverage to make sure this works as intended.

timwright_’s picture

Following this issue because we are experiencing the same problem. What I have noticed is the external link is truncated for a "icon menu" paragraph bundle. Using the nice menus module I can set the external link and it is not truncated. Confirmed downgrade to 1.6 resolves the issue.

silverham’s picture

silverham’s picture

Attached patch reverts #2428185: Language prefix and relative links broken as a hotfix for now. Needs better fix for the original issue.

summit’s picture

Hi, Thanks for this great patch as a hotfix! It was very important for me.
I use label as link, with label as title, and then I rewrite it to

<a class="btn btn-primary" href="[field_affiliate1-url]" target="_blank">[field_affiliate1-title]</a>

greetings, Martijn

silverham’s picture

The problem seems to be that us developers were never mean to use the $item['url'] directly as is.

We should of used l() or url() in the template / preprocess if doing any custom html chnages.

A quick mock up could be to use hook_preprocess_field() to get back old way and and render the $item['url'] manually before any templates. (see below).

Future work however should take these into account. @see theme_link_formatter_link_default()

/**
 * Implements hook_preprocess_field().
 */
function mythemename_preprocess_field(&$variables, $hook) {
  // Pre-render URLs from link module.
  if ($variables['element']['#field_type'] == 'link_field') {
    foreach ($variables['items'] as $key => $item) {
      $url = $variables['items'][$key]['#element']['url'];
      $link_options = $variables['items'][$key]['#element'];
      // Url function can't handle these.
      // @see theme_link_formatter_link_default().
      unset($link_options['title']);
      unset($link_options['url']);
      $variables['items'][$key]['#element']['url'] = url($url, $variables['items'][$key]['#element']);
      // Add Reset the options that would change the URL again later.
      $variables['items'][$key]['#element']['fragment'] = '';
      $variables['items'][$key]['#element']['query'] = array();
    }
  }
}
la558’s picture

@Ron Collins (post #4) and @timwright_ (post #7) how did you downgrade to 1.6?
I have valuable information on those fields, so uninstalling/re-installing this module is not an option.
Thanks
Laura

timwright_’s picture

@La558 Unfortunately there is no "official" way to downgrade. We were able to use a backup to revert the update.

The most useful I can find: https://www.drupal.org/forum/support/upgrading-drupal/2010-10-28/closed-...

ron collins’s picture

@La558

I used drush to downgrade:

drush dl link-7.x-1.6

There are no database updates between versions so that should be safe enough. Clear your caches after.

If you don't have drush available, you could also download from here:

https://www.drupal.org/project/link/releases/7.x-1.6

la558’s picture

Thank you for your comments @Ron Collins and @timwright_

Since the upgrading it to 1.7 was my last commit I reverted it.
And as the two of you experienced, going back to 1.6 resolves the issue.

damienmckenna’s picture

Status: Active » Needs review

Let's see what the testbot says.

damienmckenna’s picture

Status: Needs review » Needs work

We still need test coverage to make sure the correct expectations.

coufu’s picture

Having issues here too with URL fragments getting stripped. Rolling back to 1.6 in the meantime.

adriancotter’s picture

I have this issue to using tokens in Display Suite.

damienmckenna’s picture

Issue tags: -Needs tests

There's already test coverage for this - see LinkUiTest::testLinkCreate().

I suspect the best solution would be to adjust the overridden output via a preprocess function to add the extra class attributes.

damienmckenna’s picture

Status: Needs work » Needs review
StatusFileSize
new3.11 KB

A slight tidying of that test.

arcataroger’s picture

@silverham, thank you!! Using the url() function in my template fixed it. Is this documented anywhere? I looked through all the docs for the link module I could find, and it wasn't mentioned as far as I could tell. Is this a best practice for Drupal in general?

wylbur’s picture

Applying the patch to the 7.x-1.7, and this resolved issues with truncated links.

Thanks for quick action on this patch!

damienmckenna’s picture

Status: Needs review » Needs work

We don't want to roll back the existing changes, but we could work on improving handling for this scenario. To do that we need to start with documenting how to trigger this bug, write tests for it, and then work out how to fix the underlying problem.

Can someone please help by documenting how to configure the site in such a way that it triggers this problem? Thank you.

damienmckenna’s picture

The minor test changes were committed in #3110061: Tidy up test files, so we can focus this on fixing the actual problem.

Michael-IDA’s picture

{delete, wrong issue queue!}

jvogt’s picture

I was able to trigger the issue in Views, but not in my custom template. Here's my config.

Field

Custom link field (field_file_image_custom_link) on the Image file entity (using File entity module).

Field config

The key setting seems to be the Link Title. The issue is triggered when No Title is selected, but not with Static Title. I haven't tested the other options.

The rest of the field config is as follows: (Most of it is the default settings)

  • Absolute URL: checked
  • Validate URL: checked
  • Max length of title field: 128
  • Allow user-entered tokens: checked
  • URL Display Cutoff: 80
  • Link Target: Default (no target attribute)
  • Remove rel attribute automatically: Keep rel as set up above (untouched/default)
  • Override in WYSIWYG: checked [This option comes from the WYSIWYG module]
  • Number of values: 1

Test field value

https://www.google.com/search?client=firefox-b-1-d&q=drupal%20link

View

A table view listing Image file entities. Add a column for the custom link field and choose Title, as link (default) or URL, as link as the Formatter.

Output

The view column shows the truncated URL--https://www.google.com/search--as the title, but the full URL as the href. When I do the same with 7.x-1.6, both the title and the href are the full URL.

Custom template

I was unable to trigger the issue in my custom template (file--image.tpl.php) using entity_metadata_wrapper ($file_wrapper->field_file_image_custom_link->value()['url']).

damienmckenna’s picture

Could someone please build a small view that shows the problem and upload it here as file? That could give us something to work against to make sure we work out a fix for this problem.

bondjimbond’s picture

You don't even need a view; any node with a Link field and an affected URL will demonstrate the problem. View the node with the default display mode and you'll see that everything after the ? is cut off.

Example data: use the URL https://search.ebscohost.com/login.aspx?authtype=ip%2Curl%2Cuid&profile=...

What displays: https://search.ebscohost.com/login.aspx

damienmckenna’s picture

Issue tags: +Needs tests

Could someone please build a small Feature that includes the necessary configuration to show the problem? We can then wrap that in a test. Thank you.

bondjimbond’s picture

StatusFileSize
new1.87 KB

Feature attached. Just use the sample link https://search.ebscohost.com/login.aspx?authtype=ip%2Curl%2Cuid&profile=ehost&defaultdb=psyh when creating a node in the "Test content type", then view the node -- you'll see all the link parameters are cut off.

This is done on an extremely vanilla Drupal with no custom templates using the Bartik theme.

niles38’s picture

We are downgrading to 7.x-1.6 as well. This issue should be addressed ASAP.

skrossa’s picture

Having what seems like the same or similar truncation issue using tokens for a Link field in a Display Suite custom code field (in a Paragraph item).

For a link field named "File Source", with specific data
Title="Entered Title"
URL="https://example.com?parameters"
the following tokens get these results:

  • [paragraphs_item:field_file_source]=<a href="https://example.com?parameters">Entered Title</a>
  • [paragraphs_item:field-file-source]=Property field_file_source
  • [paragraphs_item:field-file-source:title]=Entered Title
  • [paragraphs_item:field-file-source:display_url]=https://example.com
  • [paragraphs_item:field-file-source:url]=https://example.com
  • [paragraphs_item:field-file-source:attributes]=Property attributes
danbarron’s picture

I may be way off base here since this the first time I've looked at Link's code, and it's not clear to me what the actual intention is here.

In _link_sanitize, the following change was made in commit a1870f1efcd2a0c170243fe0fcc12c8a0b9a9e41 starting at line 584:

Before: appears to just update the ['url'] property with the full (sanitized) URL built from its parts.

$item['url'] = url($url_parts['url'],
  array(
    'query' => isset($url_parts['query']) ? $url_parts['query'] : NULL,
    'fragment' => isset($url_parts['fragment']) ? $url_parts['fragment'] : NULL,
    'absolute' => !empty($instance['settings']['absolute_url']),
    'html' => TRUE,
  )
);

After: replaces the full $item object. It overwrites the original ['url'] with the base URL from the parsed parts and never actually returns the full sanitized URL.

$item = array(
  'url' => $url_parts['url'],
  'query' => isset($url_parts['query']) ? $url_parts['query'] : NULL,
  'fragment' => isset($url_parts['fragment']) ? $url_parts['fragment'] : NULL,
  'absolute' => !empty($instance['settings']['absolute_url']),
  'html' => TRUE,
) + $item;

Is the idea here to go ahead and return the parts, since they're being calculated anyway and might be of some use? Then I would recommend adding a ['base_url'] property:

// Add the parts to the $item array; won't overwrite ['url']
$item = array(
  'base_url' => $url_parts['url'],
  'query' => isset($url_parts['query']) ? $url_parts['query'] : NULL,
  'fragment' => isset($url_parts['fragment']) ? $url_parts['fragment'] : NULL,
  'absolute' => !empty($instance['settings']['absolute_url']),
  'html' => TRUE,
) + $item;

// Build the full, sanitized URL in ['url'], as before
$item['url'] = url($url_parts['url'],
  array(
    'query' => isset($url_parts['query']) ? $url_parts['query'] : NULL,
    'fragment' => isset($url_parts['fragment']) ? $url_parts['fragment'] : NULL,
    'absolute' => !empty($instance['settings']['absolute_url']),
    'html' => TRUE,
  )
);

If returning the parts is not wanted I think the code could be reverted to what it was.

I also wonder if ['html'] should be ['https'] since there is no 'html' option for url().

If I'm on the right track here I could submit a patch.

silverham’s picture

I also wonder if ['html'] should be ['https'] since there is no 'html' option for url().

This usually means l() won't check_plain() the title of a link if 'html' is set to true. Great work on trying to fix issue though. :-)

Issue summary so far:

This still seems as issue for some users....

URL inside link tag is OK.
title generated from url (title optional/no input): url is cut off. - NOT OK
URL from token_replace() - NOT OK.

Tests for 7.x-1.7:

URL: https://search.ebscohost.com/login.aspx?authtype=ip%2Curl%2Cuid&profile=...

token_replace() e.g. in a link template file.

$urls = array();
foreach ($items as $delta => $item) {
  $urls[] = token_replace("[node:field-page-quick-links:{$delta}:url]", array('node' => $variables['element']['#object']));
}
var_dump($urls);
// result: https://search.ebscohost.com/login.aspx
// Also: token_replace("[node:field_page_quick_links]", array('node' => $variables['element']['#object']));
// Is same as "Title, as link (default)" below.

Views showing a link from node (same as result below)

if rendering a node view:
(based off #31 feature)

Title, as link (default) - URL is OK, title has parameters url cut off (if no title entered of course, if same url is entered as title it is OK)
- result: title: https://search.ebscohost.com/login.aspx Link : OK
Title, as plain text - title has parameters url cut off (if no title entered of course)
- result: title: https://search.ebscohost.com/login.aspx
Host, as plain text - Ok - Not applicable.
- result: title: search.ebscohost.com
URL, as link - URL is OK, title has parameters url cut off (if no title entered of course
- result: title: https://search.ebscohost.com/login.aspx Link : OK
URL, as plain text
- result: title is OK
URL, absolute
- result: title is OK
Domain, as link - Ok
- result: title: search.ebscohost.com Link: OK
URL with the protocol removed
- result: title: search.ebscohost.com/login.aspx Link : OK
Short, as link with title "Link"
- result: title: "Link" - Link: OK
Label, as link with label as title
- result: title: "Link" - Link: OK
Separate title and URL
title: https://search.ebscohost.com/login.aspx & Link: (title https://search.ebscohost.com/login.aspx) - url is OK

alpe89’s picture

Downgrading to 1.6 works as #14 said

BrianLewisDesign’s picture

Downgrading back to 1.6 per #14 fixed the query string, but broke some other links and stripped the href. So no good.

Patch #21 did not fix URL, the question mark and query string are still stripped off. It was just a test file that was patched. @DamienMcKinna

Code suggestion #34 did work on, effectively the same as patching just the query string backwards to 1.6. @danbarron

Link module 7-x.1.7 - link.module file - line 591
Replace the full if (!empty($url_parts['url'])) code block with this:

  if (!empty($url_parts['url'])) {
    // Add the parts to the $item array; won't overwrite ['url']
    $item = array(
      'base_url' => $url_parts['url'],
      'query' => isset($url_parts['query']) ? $url_parts['query'] : NULL,
      'fragment' => isset($url_parts['fragment']) ? $url_parts['fragment'] : NULL,
      'absolute' => !empty($instance['settings']['absolute_url']),
      'html' => TRUE,
    ) + $item;

    // Build the full, sanitized URL in ['url'], as before
    $item['url'] = url($url_parts['url'],
      array(
        'query' => isset($url_parts['query']) ? $url_parts['query'] : NULL,
        'fragment' => isset($url_parts['fragment']) ? $url_parts['fragment'] : NULL,
        'absolute' => !empty($instance['settings']['absolute_url']),
        'html' => TRUE,
      )
    );
  }

Please fix this module ASAP. Working links are important. :)

damienmckenna’s picture

Title: link url truncated after last update » Query string, fragment stripped from URL
Assigned: Unassigned » damienmckenna

Finally ran into this on a client project.

damienmckenna’s picture

Assigned: damienmckenna » Unassigned

In my specific use case it turned out someone was grabbing the raw 'url' element instead of passing it through url(), so all of the other pieces were removed. I did notice that the query string and fragment are removed from the display but the URL added to the "a" tag retains the values when using the "link" formatter.

circuscowboy’s picture

Assigned: Unassigned » circuscowboy
Status: Needs work » Needs review
StatusFileSize
new860 bytes

I have run into this problem too. I can confirm the following:

  • Issue occurs with link and entity tokens
  • Issue doesn't occur with regular tokens
  • The token produced is categorized as type "text" and therefore doesn't do anything to add the query string
  • This can be seen in function _entity_token_get_token from entity module entity_token.tokens.inc

The options seem to be to add the query to the URL like with the attached patch or to figure out what entity token is doing with link and get it to do something more.

I have read over the issue that made the change to this area (https://www.drupal.org/project/link/issues/2428185#comment-9710045) and it seems like the purpose of the change was to store the original data and ensure the function always acts on those. I believe my patch still fulfills this but also adds the string to the URL.

Please let me know if there is more I should do in order to complete this issue.

Status: Needs review » Needs work

The last submitted patch, 40: link-n3095484-40.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

circuscowboy’s picture

Assigned: circuscowboy » Unassigned

So seeing the results of the tests show me that this is failing. It makes me think that the fix should probably be upstream - on entity token in my case.

Any thoughts or suggestions.

circuscowboy’s picture

I have currently solved the issue by moving the url() code into a hook_tokens_alter() to recreate the URL with the query and fragments.
This will be a fine solution for me until D7 EOL.

In my opinion this can be closed (won't fix) as this issue seems to lie in how entity token deals with the link. Perhaps the link module should identify to entity tokens that the result is a link for them to process. As I mentioned above entity token considers the link a text field.

Thanks

chegor’s picture

We are using Link 7x-1.7 with patch from comment #9

On our project, it fixed the case when we need a linked image to OpenStreetMap URLs. We use the token [node:field-osm-map-link:url]

emersonreis.dev’s picture

StatusFileSize
new489 bytes

Adding a patch that adds a new URI property so I can use it on the rule I have that redirects the node to the URL on the Link field. I am aware this is far from the final solution to the initial issue but thought I would add this here in case it can help someone else.

codebymikey’s picture

Status: Needs work » Needs review
StatusFileSize
new1.43 KB

Generated a new patch which addresses the issue with the url returning a value without the extra parameters like query string, fragment etc. (e.g. in tokens, or just as part of a regular entity_metadata_wrapper()->url_field-type call)

The property now makes use of a dedicated getter callback to resolve the correct value similar to the entity_metadata_entity_get_properties() callback.

Also updated the display URL for external paths to include the query string.

The #40 patch was applying the query strings twice in specific scenarios especially on the display url for relative paths, so I believe this patch should pass tests now, however dedicated tests might still be needed, but I'm not sure.

emersonreis.dev’s picture

I have seen the same behaviour as mentioned by codebymikey on #46, the patch from #40 duplicates the query strings in specific scenarios.

damienmckenna’s picture

Status: Needs review » Needs work

The patch still applies, we just need test coverage to make sure it works as expected.

jedihe’s picture

I found that the testbot is ignoring test classes that depend on 3rd party modules. The testbot console output for #46 shows it clearly: no tests from LinkTokenTest or LinkEntityTokenTest classes.

I just created #3208738: Missing proper test_dependencies, testbot ignoring some tests to resolve that, once that is resolved, we should be in a better position to identify how to properly test for the problem at hand.

jedihe’s picture

Just scheduled a re-test for #46, now that test_dependencies are properly declared in link.info.

damienmckenna’s picture

The tests pass, now we just need test coverage for the changes.

jedihe’s picture

Adding a test to verify that #46 is a proper fix. Interdiff is just --tests-only.patch

damienmckenna’s picture

This is great, thank you!

I think the test coverage could be refactored by extending LinkSanitizeTest instead of adding a whole extra test file for this.

jedihe’s picture

Updating the patch with feedback from #53. Interdiff against #46 is the same as the --test-only patch.

I couldn't do all test runs I wanted to for this one, due to a strange ISP issue. Let's see what the testbot says.

jedihe’s picture

Status: Needs work » Needs review

Looking good! let's please review.

damienmckenna’s picture

StatusFileSize
new4.13 KB
new4.02 KB

Some slight rearranging of the tests.

damienmckenna’s picture

StatusFileSize
new2.5 KB
new6.52 KB

Additional test coverage.

Status: Needs review » Needs work

The last submitted patch, 57: link-n3095484-57.patch, failed testing. View results

damienmckenna’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new7.33 KB
new4.75 KB

Extended the LinkSanitizeTest coverage some more.

damienmckenna’s picture

  • DamienMcKenna committed f9b82e4 on 7.x-1.x
    Issue #3095484 by DamienMcKenna, jedihe, silverham, circuscowboy,...
damienmckenna’s picture

Status: Needs review » Fixed

Back to green! Awesome work, everyone!

Anonymous’s picture

The issue doesn't seem entirely fixed. We tested version 7.x-1.9, and found an issue when we enter 2 query parameters with the same name (see param2 below):

https://www.test.com?param1=test&param2=anothertest&param2=onemoretest

It gets outputted as:
https://www.test.com/?param1=test&param2=onemoretest%20%20%20&param1=tes...

We're using "Title, as link".

damienmckenna’s picture

@tpat2: Please open a new issue and we'll look into this further.

Anonymous’s picture

Sorry for the delay, I've created a new issue:

https://www.drupal.org/project/link/issues/3210438

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

summit’s picture

Hi,
I am sorry to have to report that in my situation the query string is still not shown anymore in latest dev. and version 1.9.
I will open a new issue for this, because this issue is closed.

On version 1.7, this patch worked for me https://www.drupal.org/project/link/issues/1321482, I added this https://www.drupal.org/project/link/issues/1321482#comment-5443624

The new issue is: https://www.drupal.org/project/link/issues/3257043
Thanks for your reply there in advance.
Greetings, Martijn