File without extension fails upload

Created on 21 February 2024, 9 months ago

Problem/Motivation

Trying to upload the Apple verification file (without an extension) fails the extension validation and thus fails uploading.

Steps to reproduce

Enable the module and on /admin/config/system/apple-pay-verification attempt to upload an Apple verification file that does not have an extension in the file name.

Proposed resolution

Update to using the new FileExtension syntax without specifying a list of extensions.

πŸ› Bug report
Status

Active

Version

1.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States greenskin

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @greenskin
  • Merge request !1Resolve #3423037 "File without extension" β†’ (Merged) created by greenskin
  • Pipeline finished with Success
    9 months ago
    Total: 142s
    #100949
  • Status changed to Needs review 9 months ago
  • πŸ‡ΊπŸ‡ΈUnited States greenskin
  • πŸ‡ΊπŸ‡ΈUnited States dustinleblanc Ithaca, NY

    Thanks for the issue and MR @greenSkin!

    The merge request code doesn't raise any red flags to me, so I'll look at testing this out and if I can confirm the original bug and the fix, we'll go ahead and merge it. Might be a good case for the first actual tests for the module too (if only to satisfy my desire to write some). I appreciate this, as soon as I have a a little time to look at this more in depth I will.

  • πŸ‡ΊπŸ‡ΈUnited States dustinleblanc Ithaca, NY

    @greenSkin I tried to replicate this issue this weekend with a file named `apple-developer-merchantid-domain-association` and I had no issues uploading said file with no extension. Are you able to replicate this issue on a site with no additional modules or filesystem config/restrictions? While I do want to ensure the functionality works with the widest range of supported configurations, I want to be able to replicate the initial issue before merging a fix for it.

  • Assigned to dustinleblanc
  • Status changed to Postponed: needs info 9 months ago
  • πŸ‡ΊπŸ‡ΈUnited States dustinleblanc Ithaca, NY
  • πŸ‡ΊπŸ‡ΈUnited States greenskin

    We are on Drupal 10.2 and don't believe we are using any module that affects file extension validation, but I'll spin up a clean install and see if I can reproduce the issue.

    Note however that "file_validate_extensions" is deprecated and will be removed in Drupal 11. Maybe this is worth a D10+ release?

  • πŸ‡ΊπŸ‡ΈUnited States dustinleblanc Ithaca, NY

    @greenSkin looking ahead for deprecations does sound like a good idea. Are you able to send me the API doc that specifies this new method? I just saw this deprecation notice so I see that is what you're referring to. I just can't locate the documentation how this is supposed to work. The file that gets downloaded from stripe has no extension by default, so however we configure this, I think that is the use care we should support.

    Sorry for the late reply on this, I must have missed the notification in my inbox.

  • πŸ‡ΊπŸ‡ΈUnited States dustinleblanc Ithaca, NY

    Nevermind, the API docs refer to this: https://www.drupal.org/node/3363700 β†’

    That looks like exactly what you have here

  • πŸ‡ΊπŸ‡ΈUnited States dustinleblanc Ithaca, NY

    @greenSkin I had some time to come back and think about this more, I read some Drupal core issues and did some interweb searching, and came across this: https://drupal.stackexchange.com/questions/87146/upload-text-file-withou...

    I think what we really want is just to ensure the file uploaded is a text file, and maybe confirm it's size isn't enormous, and then we can just remove the extension check altogether because by default, this file will have no extension. I also don't recall if I included a permission in the module, but we probably want to have an explicit permission for accessing this form that is only given to trusted users.

    I think I'll look at adding a validator plugin that does what we want (only text, file not huge) and then add that. What do you think?

  • Status changed to Active 8 months ago
  • πŸ‡ΊπŸ‡ΈUnited States dustinleblanc Ithaca, NY
  • Pipeline finished with Canceled
    about 1 month ago
    Total: 100s
    #304727
  • Pipeline finished with Success
    about 1 month ago
    Total: 149s
    #304728
  • πŸ‡ΊπŸ‡ΈUnited States dustinleblanc Ithaca, NY

    Thanks for your patience greenskin, finally got a chance to test out the changes you've made and confirmed they work, merged.

  • πŸ‡ΊπŸ‡ΈUnited States dustinleblanc Ithaca, NY
Production build 0.71.5 2024