ComfyUI
ComfyUI copied to clipboard
comfyanonymous
allows user to save to subfolders
It would probably be better to just create folders if the filename_prefix contains \ or / instead of having a separate option
Also you should fix this: http://127.0.0.1:8188/view?file=main.py&subfolder=test/../../
Removed the subfolder input from the node and let the user type the path in the prefix field instead. Also fixed the path traversal on the view route and also added a check to enforce saving inside the output folder.
Seems to be working, I'll have to give it a try on windows to make sure it works there too unless someone can confirm it does.
It is working for me on Windows, however it throws the error:
Saving image outside the output folder is not allowed.
even if I am in the output folder like this test/../ComfyUI. This is technically in the output folder. So I would recommend calling normpath on the path to resolve any .. segments before checking if it's outside the output folder. I know it's an edge case, but I would still recommend implementing a fix to make it more robust, in case it causes other bugs I haven't found.
On second look, the issue isn't that it's not resolving .. segments correctly. It's that test doesn't actually exist, so it's giving an error. Once I create the test folder, it works. So, the issue is more that the error message doesn't match the issue.
Again, a minor edge case that would be fixed by normalizing the path first (since test doesn't need to exist in this case) which would resolve the need to have a different more accurate error message.
I also noticed that putting + or & characters in the path saves the image but breaks the image preview. This doesn't happen on the master branch. This is because it's not performing URL encoding, ~and it should really be using urllib.parse.urlencode~ instead of string concatenation.
Actually, it's JavaScript so it should be using URLSearchParams.
Thank you for your help and tests @jordanbtucker , added your changes with the last commit.
Rolled back the URLSearchParams modification. Since the filename retrieved from the server is something like file=img.png&type=temp this messes with the encoding of the parameters.
So I guess this won't support & and ? in filenames.
Rolled back the URLSearchParams modification. Since the filename retrieved from the server is something like
file=img.png&type=tempthis messes with the encoding of the parameters.So I guess this won't support
&and?in filenames.
When the file name contains any special characters I feel they should be url encoded?
See:
https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams#preserving_plus_signs And https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent
There was a typo in my suggestion. "/vew?" should have been "/view?". But it looks like you caught that.
Rolled back the URLSearchParams modification. Since the filename retrieved from the server is something like
file=img.png&type=tempthis messes with the encoding of the parameters.So I guess this won't support
&and?in filenames.
Are you rolling it back because LoadImage preview is broken?
LoadImage isn't working because you moved the file parameter out of the URL path and into the URL query. However, the IMAGEUPLOAD widget did not get updated for this change.
I recommend using URLSearchParams for this too, because shoving any file name into a URL without escaping it is asking for trouble.
var params = new URLSearchParams({file: name, type: "input"});
img.src = "/view?" + params.toString();
With this change and the other URLSearchParams change, both SaveImage and LoadImage work even with filenames that contain & and +.
Okay, so regarding the PreviewImage issue, you changed self.url_suffix = "?type=temp" to self.url_suffix = "&type=temp". This is making some of the image filenames to be sent to the web scripts as ComfyUI_00001_.png&type=temp instead of ComfyUI_00001_.png?type=temp.
The original code was just passing that string along to the server as /view/ComfyUI_00001_.png?type=temp, but with your changes it's requesting /view?file=ComfyUI_00002_.png%26type%3Dtemp&subfolder=.
So two problems here.
One: self.url_suffix = "&type=temp" assumes that every URL will already have a query in it (e.g. ?file=) when it won't.
Two: When the web script receives the image path, it's getting ComfyUI_00001_.png&type=temp, so when you try to get the filename, you're only looking for path segments so you're storing the filename as ComfyUI_00001_.png&type=temp. So, when you encode the filename and subfolder parameters, it thinks the filename is ComfyUI_00001_.png&type=temp. That's why it's getting encoded as ComfyUI_00002_.png%26type%3Dtemp.
One way to fix this is to restore self.url_suffix = "?type=temp" and then parse the src variable correctly so that you're detecting things like ?type=temp and adding that back into the URLSearchParams. Here's an example:
// Parse src as a URL. example.com is required because the URL
// constructor requires all URLs to be absolute.
var srcURL = new URL(src, 'https://example.com/');
// pathname starts with a / so we use substring to skip it.
var filename = srcURL.pathname.substring(1).replace(/^.*[\\\/]/, '');
var subfolder = srcURL.pathname.substring(1).replace(filename, '');
// Pass along any existing query parameters and set the file
// parameter.
var params = new URLSearchParams(srcURL.searchParams);
params.set('file', filename);
// If type is present, it's a upload or preview image;
// otherwise, set the subfolder.
if (!params.has('type')) {
params.set('subfolder', subfolder);
}
img.src = "/view?" + params.toString();
Yes this was my conclusion and solution too.
But I think that parsing the filename to eventually find a type url parameter then re-inject it is not the right way to do this.
I think the proper way would be to change what's returned by the server L840 of server.py to some kind of object { filename: "ComfyUI_00002_.png", type: "temp" }.
Thank you for the help on this once again, I appreciate.
There. I think it's much better/maintainable than manipulating strings in the frontend. Can someone take a look on windows please ?
This works for Save/Preview/Load nodes, am I missing another node using the /view route ?
Just tried saving to ComfyUI\output\Testing\ComfyUI and found that it continuously overwrites ComfyUI_00001_ on each gen. Doing a batch gen will save properly, but each newly queued gen will begin from 00001 again and overwrite them.
This behaviour ONLY happens when saving to a subfolder. It works fine in ComfyUI\output
@KazimirIskander You probably need to run update/update_comfyui_only.bat to get this new feature.
@KazimirIskander You probably need to run
update/update_comfyui_only.batto get this new feature. That did it, thanks.
Hi ! Is this available right now? (nov. 2023)
Because I had a file replaced today when I tried a prefix with subfolder like this: Moebius+VanGogh/ as I wanted the filename to be output/Moebius+VanGogh/_00001_.png and I had got an older image replaced output/Moebius+VanGogh_00001_.png corresponding to the same prefix but without the leading / that I was using before...
The folder exists, I'm under Linux, ComfyUI v?.?.? (installed from github on 2023-11-02) in a docker container with a volume bound into the output folder.
