File entity version: 7.x-2.0-beta1+16-dev (also 7.x-2.x-dev)

I am experiencing an issue when replacing a file. Here are my steps to reproduce:

  1. Configure an image field for a content type and specify the allowed filetypes (i.e. jpg)
  2. Create a new piece of content and upload an image asset and save the node.
  3. Edit that piece of content and edit the image field.
  4. Replace the file using an illegal filetype (i.e. tif). The allowed file types are showing all extensions for the image type and not honoring my field values
  5. Save the file.

This seems like a bug. I would expect that my allowed types would be restricted to the values that I have configured for that field instance. I am setting a parent issue, because the bug may have been introduced in that issue. See: https://www.drupal.org/node/2271229

CommentFileSizeAuthor
#9 Screen Shot 2018-11-30 at 7.41.27 PM.png893.46 KBKristen Pol
#9 Screen Shot 2018-11-30 at 7.41.10 PM.png222.22 KBKristen Pol
#9 Screen Shot 2018-11-30 at 7.36.25 PM.png37.5 KBKristen Pol
#9 Screen Shot 2018-11-30 at 7.36.14 PM.png1009.85 KBKristen Pol
#9 Screen Shot 2018-11-30 at 7.34.07 PM.png1.16 MBKristen Pol
#9 Screen Shot 2018-11-30 at 7.33.57 PM.png235.92 KBKristen Pol
#9 Screen Shot 2018-11-30 at 7.32.57 PM.png261.45 KBKristen Pol
#5 file_validation-2414461-5.patch1.09 KBhussainweb
#5 interdiff-3-5.txt670 byteshussainweb
#3 file_validation-2414461-3.patch1.04 KBhussainweb
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

agupta’s picture

agupta CreditAttribution: agupta commented

^^ also the max file size isn't honored.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented
Title: Allowed file types are not honored when replacing a file » File validation settings are not used when editing/replacing a file
Issue summary: View changes
Parent issue: #2271229: Allow file replacement extensions for the same file type »
Related issues: +#2271229: Allow file replacement extensions for the same file type

I am updating the title for accuracy. I am also moving the other issue for parent to related list. Strictly speaking, that is not the reason why this bug came up. You wouldn't notice it earlier because the file extension was essentially locked to the already uploaded file.

This bug actually belongs to the media module (you are probably using that widget). There is no option to replace the file using the generic file widget. However, I am leaving this issue here as the fix would have to go in both the places. I think the way forward would be to match the behavior of upload file where the settings from field instance are shown correctly. When you click edit, these settings are not used and file_entity module uses the extension from the file type. It does not even check the upload size limit. I will work on a patch for the file_entity module in this issue and open a new issue in media module for it's patch.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented
Status: Active » Needs review
Related issues: +#2562483: File validation settings are not used when editing a file
FileSize
file_validation-2414461-3.patch1.04 KB

I have already uploaded the required patch for media module in #2562483-2: File validation settings are not used when editing a file. This is the patch required for file_entity to actually set the options on the file upload element. Both these patches are required for this to work. I have tested this against my setup and found that both extensions and file size settings are correctly shown when editing a file.

Status: Needs review » Needs work

The last submitted patch, 3: file_validation-2414461-3.patch, failed testing.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented
Status: Needs work » Needs review
FileSize
interdiff-3-5.txt670 bytes
file_validation-2414461-5.patch1.09 KB

Fixing the failures (which were just notices).

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented

re-triggered tests, not sure if this patch still applies

Status: Needs review » Needs work

The last submitted patch, 5: file_validation-2414461-5.patch, failed testing. View results

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Status: Needs work » Needs review

testbot had a glitch, is fixed now, need to re-trigger the tests.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Hook 42 for Zicasso, Inc. commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
Screen Shot 2018-11-30 at 7.32.57 PM.png261.45 KB
Screen Shot 2018-11-30 at 7.33.57 PM.png235.92 KB
Screen Shot 2018-11-30 at 7.34.07 PM.png1.16 MB
Screen Shot 2018-11-30 at 7.36.14 PM.png1009.85 KB
Screen Shot 2018-11-30 at 7.36.25 PM.png37.5 KB
Screen Shot 2018-11-30 at 7.41.10 PM.png222.22 KB
Screen Shot 2018-11-30 at 7.41.27 PM.png893.46 KB

I reviewed the code and the changes seem pretty clear and I don't see any code standards issues.

I tested on simplytest.me and it appears to work as expected. I did the following based on the issue summary instructions:

  1. Used the article content type since it already has an image field configured for: png gif jpg jpeg
  2. Created a new piece of content and uploaded an image asset (jpg) and saved the node.
  3. Edited that piece of content and edited the image field.
  4. Tried to replace the file using an illegal filetype (tif).
  5. It did not accept the tif file so this is working as expected.
  6. Tried to replace the file using an legal filetype (jpg).
  7. It did accept the tif file so this is working as expected.

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Hook 42 commented
Issue summary: View changes

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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