Hi!
I have update my test env to drupal 7.20. Some images are break and some are not. Images putted in wyswigy editor by media module explorer are working, but images inserted by e.g. insert module are not working. How to add image token to existing links? Should I post it in wyswigy module or pathologic module issue queue? Or there are oder migrate paths?
Thanks for help...

Updated: 8/15 by andymartha

Problem Motivation

Drupal 7.20 introduced the itok to images for site security. The example is http://example.com/sites/default/files/styles/thumbnail/public/field/ima... (https://drupal.org/drupal-7.20-release-notes) If you have a Drupal 7.0x or 7.1x site with custom image styles, and you inserted the images into the node inline with the Insert module or similar (https://drupal.org/project/insert) they appear like http://example.com/sites/default/files/styles/thumbnail/public/field/ima... . If you upgrade your Drupal site to 7.2x and then update and save those image styles and revisit your nodes containing those images, the images will be missing as Drupal 7.2x expects the itok to be present in the URL. http://localhost:8888/drupal7x/sites/default/files/300x100_0.jpg (no image styles applied) will show up, but http://localhost:8888/drupal7x/sites/default/files/styles/newstyle/publi... will be a broken image, because it is missing the itok. Drupal 7.2x -> 7.2x upgrades tested OK with no broken images.

Drupal 8.x uses the itok (example: http://localhost:8888/drupal/sites/default/files/styles/newstyle/public/...) for images that have styles applied, although adding images via the Drupal 8 core WYSIWYG do not contain the itok. Patch #itok-1934498-68.patch was committed to core and patch #image-suppress_itok_output-86.patch applies cleanly from https://drupal.org/node/1934498 to allow the user to use non-itok images by altering the yml file, which I have tested. Hard-coding modules such as Insert do not exist for Drupal 8 yet.

Proposed resolution

Agree on using the query string of itok or not, which I believe the answer so far is yes. I believe David_Rothstein knows a lot about this issue. I thought these settings would be in the UI but I couldn't find them and could be mistaken. Backport patch from attiks to 7.x after finalizing https://drupal.org/node/1934498#comment-7481526 settings.

Remaining tasks

Since this issue deals with upgrading Drupal 7.1x sites to 8, I'm not sure of the best route people will take to upgrade from 7 to 8, so please inform me. Thanks!

User interface changes

none that I know of

API changes

none that I know of

https://drupal.org/node/1934498

https://drupal.org/node/1934568

Original report by @username

Files: 
CommentFileSizeAuthor
#26 i1923814-26.patch1.41 KBattiks
FAILED: [[SimpleTest]]: [MySQL] 39,842 pass(es), 12 fail(s), and 0 exception(s).
[ View ]

Comments

This is related to #1923834: New anti-DoS measure breaks displaying images in WYSIWYG content.
But in our install, existing images in WYSIWYG are not working. How do they get updated in your install?

I am away from my computer right now, but I assume you are using ckeditor module. I am using wyswigy module with media module button. In my wyswigy-source view i was able to see markup generated by media module, no src markup, but with file id insted.

I wasn't able to reproduce any problem with existing images after the upgrade (for similar reasons as described in #1923834-2: New anti-DoS measure breaks displaying images in WYSIWYG content).

However, I am seeing some issues with using Insert module on new content once Drupal 7.20 is already installed. Looking into that further.

Looks like the problem I experienced is already covered by #1923336: Insert module doesn't work with Drupal 7.20...

First, David thank you for participate in this support request.

#3 do you mean if I have existing img links in content, in body field, <img src="http://dummyhost.com/sites/somesite/files/styles/medium/public/images/regiony/pca_alfa.png> after update to drupal 7.20 it should automatically add itoken to src of an image <img src="http://dummyhost.com/sites/somesite/files/styles/medium/public/images/regiony/pca_alfa.png?itok=Pez2n9jO">?

Nope, I actually meant that it shouldn't need to add the token at all, since http://dummyhost.com/sites/somesite/files/styles/medium/public/images/regiony/pca_alfa.png will already exist in the filesystem. Someone trying to visit that URL should be able to access the image without Drupal ever being involved.

The only way I can think of where that wouldn't happen is:

  1. Someone inserts an image using Drupal 7.19, but the content which contains the inserted image never gets viewed (and thus the image is never generated).
  2. After that, the site is upgraded to Drupal 7.20.
  3. After that, the content is viewed. The image will at that point fail to be generated.

This is theoretically possible, but I have trouble seeing how it would happen in practice (especially since after you create content you get redirected right away to view it and thus the content created with Drupal 7.19 should have had its images viewed and generated right away). Maybe if the image is inserted in a paritcular textarea that is not displayed by default with the content, but only in rare cases?

Do you have a set of steps to reproduce this in practice?

Status:Active» Postponed (maintainer needs more info)

Status:Postponed (maintainer needs more info)» Closed (works as designed)

Thank you for explanation David.
I am using File (Field) Paths and insert modules, I think my issue is releated with http://drupal.org/node/1923336 and http://drupal.org/node/1925298. I will close this one...

Title:existing images after update to drupal 7.20Existing hardcoded images can break after update to drupal 7.20
Status:Closed (works as designed)» Active

Hi @David_Rothstein
The issue is a bit more complected then #7

1) Someone inserts an image using Drupal 7.19
2) After that, the site is upgraded to Drupal 7.20.
3) Image styles are changed or the stored values are cleared.

1 & 2 - are a common use case
3 - can easily happen especially if you make changes to a preset. That can make the "Update Style" button deadly.

If you update a style after upgrading to 7.20 you will have a noticeable issue, however if you update it before upgrading to 7.20 you may have a number of missing style images that will not be generated after upgrading (harder to spot).

The two options to resolve this are:

1) Set the image_allow_insecure_derivatives to TRUE spider the site and set it back to FALSE. and remember to do this if you ever need to update an image style.

2) Use a content filter together with your input format @pp posted a sandbox module Old Image link corrector (didn't get a chance to test it yet).

Although the issue may be caused by other modules like the insert module and those modules are addressing this issue buy updating the link creation functionality, they are not addressing the existing content issue.

I think this is a core issue but cant really offer a good solution except perhaps for core to provide a backward compatibility content filter and providing good instructions on this issue and how to resolve it. This will allow users to fix the issue by simply checking a box on the "Enabled filters" section of the relevant text format.

I am reopening and renaming the title to be more specific to this issue, however if you think this is not the responsibility of core or that core can't provide a reasonable solution feel free to change it back.

Title:Existing hardcoded images can break after update to drupal 7.20Existing hardcoded images can break after updating to Drupal 7.20 if image styles have been re-saved
Version:7.20» 7.x-dev
Category:support» bug

3) Image styles are changed or the stored values are cleared.
...
3 - can easily happen especially if you make changes to a preset. That can make the "Update Style" button deadly.

Ouch, yes, that looks like a problem. Thanks. Anything that causes image_style_flush() to be called could cause this issue, and that includes re-saving image styles in the user interface.

For what it's worth, there seems to be a version of this problem that exists in Drupal 7.19 only as well: Edit an image style and change its name. Any hardcoded links will point to the old, invalid image style name and will fail. (Though perhaps that part is up to #606598: Human readable image-style names to solve.) In any case, Drupal 7.20 makes it much worse since now any change you make can have the same effect.

I am not sure what to do to fix this but we probably need to do something. The text filter is an option of last resort but might be too broad to be generally safe; maybe it can be worked on though. Other ideas could include (a) a batch process to update existing images rather than deleting them when a style is re-saved (could be very heavy though), or (b) record somewhere in the database that the images once existed before deleting them during the flush, and don't require a token to regenerate them later. Neither of these sound like great ideas though.

I have added this issue to http://drupal.org/drupal-7.20-release-notes.

@David_Rothstein yes no easy solution
I don't think (b) will work as existing sites can have some missing before an upgrade and it will add an overhead to people that don't have this issue.
(A) may work but will need to potentially scan the schema for candidate fields or a UI for the user to chose fields to update, complicated.
and at the end of the day (a) will need to use the same logic as a text filter for identifying and converting the links.
I know the text filter is not ideal but at the moment can't think of a better way, it should add no significant overhead, if enabled will have its output cached.

If we go with a text filter our options are:

  1. Provide it in core: Users encountering this issue just need to edit the relevant text format and check the filter.
  2. Provide in core as an optional module: Users encountering this issue need to enable a module, edit the text format and check the filter.
  3. Provide as a contrib module: Users encountering this issue need to download and enable a module, edit the filter and check the filter.

Maybe someone can come up with other ideas.

https://drupal.org/sandbox/pp/1924318 if we go with text filter idea.
There is great markup generated by the media module + wyswigy module, if we would have such a thing in core before drupal 7.20 released there would be not any problem.
Please read my #1 post from https://drupal.org/node/1924618.

@David_Rothstein:

Is your issue with IMCE the same as #1930698: drupal 7.20 update broke image style preset generating thumbs?

I'm trying to figure out what component in my chain should be responsible for handling it.
In my case:

  • BUEditor has markdown_editor plugin,
  • Markdown_editor plugin uses IMCE dialog and its own JS to style it,
  • IMCE dialog allows to select derived image that will be inserted into content:
    • either directly by navigating to it;
    • or by returning modified path after selecting style from menu (my local change in markdown_editor JS),
  • IMCE dialog returns URL which is then embedded into content as a Markdown tag with reference to image,
  • Markdown content filter grabs the markdown image tag and changes it into img.

Statuses:

  • Markdown filter maintainter thinks that such functionality belongs in a separate generic module but he is open to suggestions and patches.
  • BUEditor maintainer assumes that derivate images are always present if one can pick up their paths.
  • Don't know if it's about IMCE, but there is an issue open for it.

Maybe I should write some small content filter for such cases as mine?
Something that would parse HTML and add itok token for any image path found, that looks like derivate.

Even if IMCE could generate paths with tokens that won't solve the problem with hardcoded paths (in case of Markdown it is a blocker since Markdown syntax allows you to refer to paths by hand – that's one of its features).

One other issue is the WYSIWYG editor on the edit form that's cant be fixed by a text filter

Simple redirector would make life easier for module developers.
If image does not have itok attached and user is authenticated then redirect to URL with itok.

Other variant of it is simple "service" that takes image URL and returns tokenized version. That could be used in JS to get it and alter something permanently.

I can confirm the issue described in #7 and #10 - we have a drupal commerce shop with 10,000 products in, I changed one or more image styles shortly before upgrading to 7.20 so would have had 1,000s of product pages that didnt have the images generated before the update.

The images were added using the core image module as they are file type image field on the commerce product.

I tried several fixes including micz_ sandbox module (I realise now this only fixes images inserted with WYSIWYG), also tried the image style flush module, drush cache clear all but ended up rolling back to 7.19 and all is working again.

This does need a fix for the core image module, I can test any fixes put in place, will monitor.

Thx - kevster

#18, kevster: it is pp's sandbox module, not mine. It is working with images inserted with wysiwyg only.

If image does not have itok attached and user is authenticated then redirect to URL with itok.

Wouldn't need a redirect for this, it'd be possible to just allow authenticated users to regenerate images without the token.

This is still a potential security issue for sites with open registration, but could do a couple of things to help with that as well:

- use the flood module for token-less image derivative creation to limit the number any one account can regenerate per hour.

- add a new permission so that the behaviour can be enabled/disabled per site similar to the $conf setting now.

Since most Drupal sites allow anonymous users to access content, that won't help too much in general, will it? (Any site experiencing this issue would still have anonymous users see the broken images until an authenticated user happened to come along and generate them.) But maybe it makes sense in some scenarios.

For the issue being discussed in comment #10 onward, it sounds like building off https://drupal.org/sandbox/pp/1924318 is the best option. It makes more sense to me in contrib personally (especially if sites using the Insert module are by far the most likely to encounter the problem), but certainly open to discussing it for core as well.

@kevster:

The images were added using the core image module as they are file type image field on the commerce product.

The issue described in comment #10 should not occur with image fields, since they generate the URL on the fly (when the field is displayed), and it will contain the correct token to regenerate the image. It should only occur with hardcoded URLs stored in textareas.

If you're experiencing problems with image fields too (basically, if your server is not generating new image derivatives at all under any circumstances) I think it's a different problem. See #1109312-69: image styles not creating files (no thumbnails or custom styles) on some servers for a possible explanation.

#19 - sorry micz_ yes its pp's module.

#22 Hi David - we are experiencing this but the server is ok re making new image styles based images - after rolling back to 7.19 it is generating images fine for the core image field based fields on commerce products. We arent really using the WYSIWYG image input method much as its a shop so mainly focussed on the product pages.

When I viewed the source code of missing images that hadnt been viewed before the 7.20 upgrade they didnt have the token added so didnt display but some other older ones were ok and had the token appended.

Im happy to stay on 7.19 for now but would be interesting to see if anyone else runs into the same scenario...

@David - you could allow images without the token to be generated by anonymous users with flood control as well.

It's going to mean some images stay broken if there's a lot, but they'll eventually get fixed, while still preventing a DOS.

Anyone experiencing this issue might want to try out #1934568: Allow sites using the 'image_allow_insecure_derivatives' variable to have partial protection from the Drupal 7.20 security issue, which if possible I am hoping to release today (Wednesday). It won't fix the issue directly, but it makes the 'image_allow_insecure_derivatives' variable safer to use (although still not completely safe) as a temporary workaround.

@catch, yeah, we discussed the flood control possibility a bit in the security team. To protected against a distributed denial-of-service attack, the flood control would have to be sitewide (not based on IP address), but that is possible. After an image style is flushed, there will be many new requests for image derivatives expected, which might look like a denial-of-service attack even if it isn't, so there's a good chance that people will be denied access in some cases. Nonetheless, I agree it couldn't hurt as a backup method for the token, even if it isn't 100% reliable.

@kevster, you are saying that you are using the core image field, but tokens are still not appearing in the HTML source when that field is displayed? What display formatter (on the "Manage Display" screen) is being used to display this field?

Status:Active» Needs review
StatusFileSize
new1.41 KB
FAILED: [[SimpleTest]]: [MySQL] 39,842 pass(es), 12 fail(s), and 0 exception(s).
[ View ]

If I understand correctly to only problem is that you can derivatives of already created image styles, so why don't we just check if somebody is trying to do just that.

I created a quick patch demonstrating my idea, if this is a possible solution we need to do some clean up. On the other hand if I'm missing something, ignore this ;-)

Status:Needs review» Needs work

The last submitted patch, i1923814-26.patch, failed testing.

Status:Needs work» Needs review

Test failures are expected, since tests are still using the token

Will we won't fix it because 7.21 arrived, or postpone it on a non token solution?

@catch, yeah, we discussed the flood control possibility a bit in the security team. To protected against a distributed denial-of-service attack, the flood control would have to be sitewide (not based on IP address), but that is possible.

Yeah it won't handle every case, but seems like a decent fallback to have. We can also make the flood limits configurable.

This issue got me today after upgrading a 7.19 site to 7.20, then changing some imagecache settings. Thanks at least for the fix in comment #10, that did allow the images to regenerate.

Thanks!
--Tony

Version:7.x-dev» 8.x-dev
Priority:Major» Critical
Issue tags:+needs backport to D7

The original security fix was committed to 8.x, so moving this there and we can backport once it's in.

Also bumping priority this is going to be a problem for any 7.x site with hard-coded images that upgrades to 8.x as well, not only 7-7.

So the above patch is similar to what was released in Drupal 7.21 via #1934568: Allow sites using the 'image_allow_insecure_derivatives' variable to have partial protection from the Drupal 7.20 security issue, except we only used that restriction on certain sites (as a backup security measure in the case where a token is missing, and only for sites that have opted out of the more comprehensive security protection).

I don't think we should prevent recursive image generation entirely, though. If it's authorized it should be allowed. And I don't think we can have that check as a substitute for the tokens either, because the tokens prevent against other kinds of denial-of-service attacks too.

So looking at the other two proposals on the table, it seems like they are:

  1. Flood control/thresholds for the number of unauthorized images that can be generated without a token in a given period of time.
  2. A text filter to add tokens to hardcoded links that don't have them.

It occurs to me now that a possible problem with #1 is that although it will protect against a standard denial-of-service attack, it won't necessarily protect against someone trying to (slowly) fill up your hard disk with unwanted images. Let's say your site has 50,000 images in the files directory and 10 image styles. That definitely doesn't mean you have things set up with the expectation that all possible combinations (500,000 of them) can be generated. If someone does this over a period of time that could easily result in your hard disk filling up which could cause your site major headaches.

"I don't think we should prevent recursive image generation entirely" maybe somebody will need this, but the impact will be a lot less than the current token implementation, so we could make the check optional (default on), but overridable with a $config setting "image_allow_recursive_derivatives".

Flood control will be hard to implement, using your example from #33: If no derivative is created, you cannot really set the flood control: suppose I have a gallery page with 60 thumbnails and 60 full images, this means all 120 of them should be generated as fast as possible. On the other hand, once all derivatives are generated I probably would want to lower the flood setting (not 120 / second anymore).

A text filter might work for the wysiwyg, but this will still not solve the problem with javascript changing an img src, because the token isn't available.

Small note about your example: this is not your average Drupal site, so it is probably running on some decent server (I hope) with enough storage.

If you have a gallery page all of those should have the token and not hit flood control, the only images that wouldn't would be those hard-linked in HTML somewhere (text filter or a poorly coded theme). The poorly coded theme and/or text formats that don't have that filter enabled is what makes me think that a filter isn't enough by itself.

To prevent the disk usage issue we could also have a counter that resets when image styles are flushed, then places a global limit on the number of 'invalid' derivatives created after that - after this start logging to watchdog and/or a message in hook_requirements() - can set that to 1,000 configurable and that'd either alert people to a potential denial of service attempt or that they have this issue on their site and need to explore other options.

#35 You're right, those would have a token, unless you're using something like resp_img where you have to disable it, because otherwise the module no longer works (it uses js to replace parts of the img src).

Status:Needs review» Active

#36 in that case it would be easy to add meta data into the tag (like data-style1-token, data-style1-src etc.) other that module is not compatible to Drupal 7.20+.

Back to active as patch #26 was for D7 and for a feature that has been included in 7.21.

#37 I know it *can* be solved, it only isn't much fun to discover it this way.

Related issue: #1934498: Allow the image style 'itok' token to be suppressed in image derivative URLs which is a way better way to solve this (EDIT: 'way better' if we can get rid of the token)

#38 Sure! That why I created that issued. I hope we can quickly fix that and simply close this issue.

Confirming this issue as well.

1. I have updated from 7.17 to .21
2. Created an image style few hours before. Around 20% of necessary thumbnails were created (they are used on seldom visited pages) before the upgrade.
3. After the upgrade, I received access denied errors when trying to generate thumbnails.
4. Tried to generate them using different Views, didn't work.
5. Created totally new image style to deal with any images, but couldn't get a single one to work.
6. I'm uploading images through core field.
7. Switching back to 7.19 solved the issue.

When you have Thousands of images many times an image is not generated for a certain image size's for months so my site's watchdog blew up:(

One other issue to consider although not directly connected to "Existing hardcoded images" is that Google may have indexed images on your site and those would be indexed without the token and will get an access denied after the update.
Hopefully Google will re index the images (with the token) over time but this may be an issue for some business types.

Since switching back and forth between 7.19 and 7.21, I think google is going to have a heart attack on my site. Not that I care much for the new and "improved" version of google images. And yes I had this issue!

Hi @MGParisi if you are having issues with 7.21 you can allays use the "image_allow_insecure_derivatives" flag instead of rolling back to 7.19. This will protect you from the worst possible attack and will keep Google happy.
you can set the flag in the flag in the settings.php:
$conf['image_allow_insecure_derivatives'] = TRUE;

$conf['image_allow_insecure_derivatives'] = TRUE; did not work for me unfortunately. Tried it on two of my websites. I'm on Debian with Nginx if that means anything. I'm considering going back to 7.19 since images were working fine before the upgrade to 7.20.

I also used $conf['image_allow_insecure_derivatives'] = TRUE; and so many modules where broken and my watchdog was spitting out errors left and right I also had to role back.

Broken Modules
Fivestar
Hover Preview
Zero Points Drop Down Menu
ImageMagick

... All had issues (and was not listed). There are issues open for each one! Reverting back fixes all issues. 7.20 and 7.21 are the first Drupal core updates I have had issues with and they break things pretty badly. I also have the issue in this discussion because of the situation I mentioned in my post above.

Priority:Critical» Major

Demoting to major, since #1934498: Allow the image style 'itok' token to be suppressed in image derivative URLs will probably provide a solution for this as well.

Priority:Major» Critical

I just reviewed the current patch in that issue, and I don't see how it affects this one?

If the other were to be committed, the easiest way to fix this one is to enable the suppress_itok_output option?

I thought enabling the existing 'image_allow_insecure_derivatives' option already lets you work around this?

I think what we're looking for in this issue is a solution that works without setting any of those variables.

http://drupal.org/node/1427826 contains instructions for updating the issue summary with the summary template.

The summary has never been revised, so check the comments for possible updates.

Hi!

I have a problem with the Hoover Preview module and I don`t get any response in the forum for that module so I`m trying a different approach and try to find someone describing a nearly similar problem.

I have some problems with displaying the preview images. It looks ok if I set the original image as preview image, but fails with any of the custom ones. With those I just get a progress symbol spinning around and around.

This is just a starting input.

Can anybody suggest what I should do?

@klausenlie: Although your issue in #1932340: Problems after the upgrade to 7.20 is related to this, it will be up to the Hover Preview module's maintainer to update the module to be compatible with the current release of D7.

I updated the issue summary to hopefully clarify & push this forward, will accept criticism.

Downgrading this to major - unless this is going to block the next point release of Drupal 7 I don't think it should block 8.0 either. There's no data loss nor fatal errors, and there's an existing workaround.

Priority:Critical» Major

Issue summary:View changes

directed by XJM at Midwest Drupal Summit 8/15/13