ComfyUI icon indicating copy to clipboard operation
ComfyUI copied to clipboard

Workflow json file path doesn't get URL-decoded.

Open TobTobXX opened this issue 1 year ago • 6 comments

Expected Behavior

When I save/load a workflow from userdata it gets saved in (or loaded from) <userdir>/workflows/name.json.

Actual Behavior

When I save/load a workflow from userdata it gets saved in (or loaded from) <userdir>/workflows%2Fname.json. (%2F is a slash (/) in URL-encoded format).

Steps to Reproduce

Save a workflow and inspect the userdata dir. Or load a workflow saved in the workflow directory and observe a 404.

Debug Logs

(The terminal doesn't produce an output when triggering the bug.)

Other

ComfyUI: v0.2.0 ComfyUI Frontent: v1.2.40

TobTobXX avatar Sep 02 '24 17:09 TobTobXX

The relevant code sections are here:

The frontend API adapter encodes the file path with URL-encoding:

  /**
   * Gets a user data file for the current user
   */
  async getUserData(file: string, options?: RequestInit) {
    return this.fetchApi(`/userdata/${encodeURIComponent(file)}`, options)
  }

  /**
   * Stores a user data file for the current user
   * @param { string } file The name of the userdata file to save
   * @param { unknown } data The data to save to the file
   * @param { RequestInit & { stringify?: boolean, throwOnError?: boolean } } [options]
   * @returns { Promise<Response> }
   */
  async storeUserData(
    file: string,
    data: any,
    options: // [...]
  ): Promise<Response> {
    const resp = await this.fetchApi(
      `/userdata/${encodeURIComponent(file)}?overwrite=${options.overwrite}`,
      {
        method: 'POST',
        body: options?.stringify ? JSON.stringify(data) : data,
        ...options
      }
    )
    // [...]
  }

https://github.com/Comfy-Org/ComfyUI_frontend/blob/e733b87f2229f0c23dd8d2096ae8dcc04fa7855e/src/scripts/api.ts#L552

The backend API adapter appears to NOT URL-decode the file path:

        def get_user_data_path(request, check_exists = False, param = "file"):
            file = request.match_info.get(param, None)
            if not file:
                return web.Response(status=400)
                
            path = self.get_request_user_filepath(request, file)
            if not path:
                return web.Response(status=403)
            
            if check_exists and not os.path.exists(path):
                return web.Response(status=404)
            
            return path

        @routes.get("/userdata/{file}")
        async def getuserdata(request):
            path = get_user_data_path(request, check_exists=True)
            # [...]

        @routes.post("/userdata/{file}")
        async def post_userdata(request):
            path = get_user_data_path(request)
            # [...]

https://github.com/comfyanonymous/ComfyUI/blob/d043997d30d91ab057f770d3396c2e288e37b38a/app/user_manager.py#L137 (apparently, I'm not well-versed in python webserver frameworks).

TobTobXX avatar Sep 02 '24 17:09 TobTobXX

I submitted a PR that fixes this (tested). However, I want someone to really scrutinize this, since I don't know what caused it and it could break something. ~I haven't seen any errors yet though.~ EDIT: This isn't quite true. I get 404s on workflows/.index.json, but this file really does not exist, so that makes sense.

TobTobXX avatar Sep 02 '24 17:09 TobTobXX

Man i've been trying to figure this out for like 3 hours. I wish I came to the github issues sooner. Here's a quick fix that works. Not sure if the devs want it done this way but it works for now.

Create the .index.json file in user/default/workflows/ and add the following:

{"favorites":[]}

in app/user_manager.py add:

import json
import os
import re
import uuid
import glob
import shutil
from aiohttp import web
+ from urllib import parse
from comfy.cli_args import args
from folder_paths import user_directory
from .app_settings import AppSettings

default_user = "default"
users_file = os.path.join(user_directory, "users.json")
#[...]

    def get_request_user_filepath(self, request, file, type="userdata", create_dir=True):
        global user_directory
        
        if type == "userdata":
            root_dir = user_directory
        else:
            raise KeyError("Unknown filepath type:" + type)

        user = self.get_request_user_id(request)
        path = user_root = os.path.abspath(os.path.join(root_dir, user))

        # prevent leaving /{type}
        if os.path.commonpath((root_dir, user_root)) != root_dir:
            return None

        if file is not None:
            # prevent leaving /{type}/{user}
+           if "%" in file:
+               file = parse.unquote(file)
            path = os.path.abspath(os.path.join(user_root, file))
            if os.path.commonpath((user_root, path)) != user_root:
         #[...]

The user/ directory should be in this structure:

CleanShot 2024-09-02 at 19 22 51@2x

That should fix the error and allow you to save and load workflows.

w1gs avatar Sep 02 '24 23:09 w1gs

Create the .index.json file in user/default/workflows/ and add the following: [...]

Wait... is .index.json actually used anywhere?

       if "%" in file:
           file = parse.unquote(file)

that's exactly what I changed in the PR, except that I didn't add the if-condition. If the string contains no URL-encoded characters, the function is a no-op, so the condition isn't needed.

TobTobXX avatar Sep 03 '24 10:09 TobTobXX

Create the .index.json file in user/default/workflows/ and add the following: [...]

Wait... is .index.json actually used anywhere?

       if "%" in file:
           file = parse.unquote(file)

that's exactly what I changed in the PR, except that I didn't add the if-condition. If the string contains no URL-encoded characters, the function is a no-op, so the condition isn't needed.

From what I see, the web ui tries to load the favorited workflows from that .index.json file. I'm not sure what else reads that file though.

The if condition was added since I was getting errors trying to unquote the filename whenever there wasn't a "%" in the filename. This could all just be related to how I have my menu bar setup. I have it set to "top" which seems to be a Beta feature at the moment. CleanShot 2024-09-03 at 10 28 47

w1gs avatar Sep 03 '24 14:09 w1gs

Same issue, thought it was due to ComfyUI-Custom-Scripts

Create the .index.json file in user/default/workflows/ and add the following: [...]

Wait... is .index.json actually used anywhere?

       if "%" in file:
           file = parse.unquote(file)

that's exactly what I changed in the PR, except that I didn't add the if-condition. If the string contains no URL-encoded characters, the function is a no-op, so the condition isn't needed.

From what I see, the web ui tries to load the favorited workflows from that .index.json file. I'm not sure what else reads that file though.

The if condition was added since I was getting errors trying to unquote the filename whenever there wasn't a "%" in the filename. This could all just be related to how I have my menu bar setup. I have it set to "top" which seems to be a Beta feature at the moment. CleanShot 2024-09-03 at 10 28 47

Might be due to that

cyber827 avatar Sep 03 '24 20:09 cyber827