Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Illegal filename #347

Closed
eight04 opened this issue May 23, 2024 · 21 comments · Fixed by #353
Closed

Illegal filename #347

eight04 opened this issue May 23, 2024 · 21 comments · Fixed by #353
Labels

Comments

@eight04
Copy link
Owner

eight04 commented May 23, 2024

https://bugzilla.mozilla.org/show_bug.cgi?id=1898498

Should we create a test to find all illegal characters?

@eight04 eight04 added the bug label May 23, 2024
@Djidam
Copy link

Djidam commented Jun 13, 2024

Since the last update of firefox (v127.0), firefox throws an illegal character error when downloading files, and so nothing is downloaded.
I use this pattern for naming the files : ${pageTitle}/${index}.jpg

I use another addon, it uses pagetitle for naming the file, but no error with the last update of firefox (https://addons.mozilla.org/en-US/firefox/addon/save-text-to-file/) ?

@eight04
Copy link
Owner Author

eight04 commented Jun 13, 2024

save-text-to-file seems to use a native app to write data to the disk so it won't be affected by Firefox update.

@Djidam
Copy link

Djidam commented Jun 13, 2024

I have not install that local app. By default, files are saved in the download directory used by Firefox. If you wish to save them elsewhere, then you need the local app.

@eight04
Copy link
Owner Author

eight04 commented Jun 13, 2024

image
I can also reproduce this bug with save-text-to-file.

Maybe the filename doesn't include illegal characters when you save the text?

@Djidam
Copy link

Djidam commented Jun 13, 2024

Hmm
On the website I use both addons, one for text, yours for images (love it actually). Both with pageTitle. The pageTitle has those 3 characters . - !
The file for the text is saved, but I get the error for the images directory.
I'll use your addon in chrome, I tested my setup and that will do the job.

@eight04
Copy link
Owner Author

eight04 commented Jun 16, 2024

What is your OS and Firefox version? I can't reproduce on Windows 10 + Firefox 129.

BTW, you can preview the filename by hovering on the file pattern in batch download:
image

@eight04
Copy link
Owner Author

eight04 commented Jun 16, 2024

@Djidam
Copy link

Djidam commented Jun 17, 2024

Windows 10 and firefox 127.
I tried to figure out what causes the error and I managed to reduce the string to: 1. A\1.jpg
I don't know why but if I have spaces after a dot, then firefox throws an error (the space can be right after the dot or it can have characters between, like 1.1 A\1.jpg)
image
image

@brazenvoid
Copy link

I am also having this issue with the character | in the page title. It used to work completely fine before but now any page title with an illegal character does not work.

@brazenvoid
Copy link

Correction it is as @Djidam says, it only fails when there is a dot and a space in the name! How odd!

@eight04
Copy link
Owner Author

eight04 commented Jun 23, 2024

Link to the dot and space bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1903780

@brazenvoid
Copy link

@eight04 isn't there some way you can make your users vote on the report at bugzilla, like put a message on the addon page or something so it gets prioritized? I don't think the single vote that I added would make a difference...

@brazenvoid
Copy link

If someone wants a workaround, a simple userscript can do the job with one line of code. Here is what I am using:

// ==UserScript==
// @name         Firefox Download Bug Workaround
// @namespace    brazenvoid
// @version      1.0.0
// @description  Workarounds the download bug that does not allow a dot and a space in the filename
// @author       brazenvoid
// @match        *
// @grant        none
// ==/UserScript==

document.title = document.title.replaceAll('. ', ' ')

@Djidam
Copy link

Djidam commented Jul 10, 2024

If someone wants a workaround, a simple userscript can do the job with one line of code. Here is what I am using:

// ==UserScript==
// @name         Firefox Download Bug Workaround
// @namespace    brazenvoid
// @version      1.0.0
// @description  Workarounds the download bug that does not allow a dot and a space in the filename
// @author       brazenvoid
// @match        *
// @grant        none
// ==/UserScript==

document.title = document.title.replaceAll('. ', ' ')

Hi, I tried your script with tampermonkey, it doesn't work for me. Maybe I'm missing something ?

I finally opted for a portable version 126 of firefox (had troubles with chrome) and it does the job.

@brazenvoid
Copy link

brazenvoid commented Jul 10, 2024

@Djidam Well it works for my needs. I did update it a bit:

// ==UserScript==
// @name         Firefox Download Bug Workaround
// @namespace    brazenvoid
// @version      1.0.2
// @description  Fixes the download bug that does not allow a dot and a space in the filename
// @author       brazenvoid
// @match        https://[your site]/*
// @grant        none
// ==/UserScript==

document.title = document.title.replaceAll('. ', ' ').replaceAll('%', '').replaceAll('.', '-')

Replace [your site] with the domain you want it to work on or * as in the first version.

@hdd113
Copy link

hdd113 commented Jul 17, 2024

Until the issue is fixed, perhaps we could add an option that strips all special characters from the pageTitle? Even apart from the bug it could be useful if you want to remove any potentially illegal characters from the path in case you want to set up a further pipeline that runs on the computer after the download.

@brazenvoid
Copy link

brazenvoid commented Jul 17, 2024

Yeah, it does seem that the Firefox developers don't really have any input from extension API devs when making changes in their internal functions especially when they are shared with them.

The bug discussion has now gone to figuring out how or to even make provision for the extension API's specific needs. They make it sound as if the extension developers were using it wrongly after they themselves changed the scope of the problematic internal function.

It is eerily similar to the blame games I experience among developers when they are caught being irresponsible.

@brazenvoid
Copy link

@eight04 Can you please mention in the bugzilla thread for dot and space bug, what you want from the download.sanitize API. The developer thinks that you only want directory validation and not file validation.

Quite an odd understanding in my opinion.

I think that you want complete path validation including the directory and filename.

@eight04
Copy link
Owner Author

eight04 commented Jul 19, 2024

strips all special characters from the pageTitle?

It is possible to do something like

Image Picka/{pageTitle.replace('.','_')}/{...

Since %, spaces and dots are usually valid characters, I'm still not sure if we should escape them by default.

@eight04
Copy link
Owner Author

eight04 commented Jul 19, 2024

@brazenvoid I think they are going to the right direction.

They will need 2 API, one for validating filename and one for folder name.

Now we only have one API. It is used to validate both filename and folder name. After ff127, it gets more strict to file extensions by disallowing spaces. While it makes no sense to folder name since there is no extension.

It seems that they want to create a new option in the API to support both cases.

@eight04
Copy link
Owner Author

eight04 commented Jul 19, 2024

Another case that may also lead to an invalid filename:
When the filename includes a dot and a space while the the filename is too long and get trimmed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants