Skip to content

Patch lua-spore to properly set filename in form-data#2123

Merged
Frenzie merged 1 commit into
koreader:masterfrom
jacopo-degattis:fix/lua-spore-filename-form-data
Jul 10, 2025
Merged

Patch lua-spore to properly set filename in form-data#2123
Frenzie merged 1 commit into
koreader:masterfrom
jacopo-degattis:fix/lua-spore-filename-form-data

Conversation

@jacopo-degattis

@jacopo-degattis jacopo-degattis commented Jul 8, 2025 •
edited by Frenzie
Loading

Copy link
Copy Markdown

During the development of a koreader plugin I encountered the following problem using the lua-spore library.

I have an api.json file like this

"upload_file": {
      "path": "/api/v1/files/upload",
      "method": "POST",
      "required_params": [
        "file",
        "libraryId",
        "pathId"
      ],
      "expected_status": [
        200,
        201,
        400,
        414,
        500
      ]
      "form-data": {
        "file": ":file",
        "libraryId": ":libraryId",
        "pathId": ":pathId"
      },
}

When I invoke the upload_file method like this

client:upload_file({
  file = "@/absolute/path/to/file.txt",
  libraryId = "1",
  pathId = "1"
})

The request is succesfully sent but the server is unable to save the file locally because of a problem in the way the form-data is built.

Until now when you use an absolute path inside the file field the lua spore library would send a form-data with the filename set exactly to the absolute path you specified while instead it should just put the filename of the corresponding file.

This fix aims to resolve that problem and simply by getting the last part of the path I'm able to correctly set the filename field inside the form-data and correctly save the file on the backend.

So we move from something like this

Content-Disposition: form-data; name="file"; filename="/absolute/path/to/file.txt"\r\n'

to

Content-Disposition: form-data; name="file"; filename="file.txt"\r\n'

This change is Reviewable

@jacopo-degattis

jacopo-degattis commented Jul 8, 2025

Copy link
Copy Markdown
Author

Reference issue: #2122

@NiLuJe

NiLuJe commented Jul 8, 2025

Copy link
Copy Markdown
Member

What if the filename can and should contain a subfolder?

On mobile right now, and a bit rusty, but that pattern only keeps the final path component, right?

@jacopo-degattis

jacopo-degattis commented Jul 8, 2025 •
edited
Loading

Copy link
Copy Markdown
Author

Yes the pattern only keeps the final component but the filename inside the form-data should be just a reference to what the file should be called on the server and shouldn't store the path of the file; filename is just a suggestion on how the file should be called on the server.

So you actually provide a full path to the lua-spore method in order to fetch the file binary data but inside the form-data you just provide the filename as an information to tell the server how to name that file.

@NiLuJe

NiLuJe commented Jul 9, 2025

Copy link
Copy Markdown
Member

Ah, okay ;).

LGTM then ;).

Frenzie
Frenzie reviewed Jul 9, 2025
View reviewed changes
Comment thread thirdparty/lua-Spore/form-data-filename.patch Outdated
@Frenzie Frenzie merged commit 3812fe9 into koreader:master Jul 10, 2025
4 checks passed
@jacopo-degattis jacopo-degattis deleted the fix/lua-spore-filename-form-data branch July 10, 2025 09:14
@Frenzie

Frenzie commented Jul 15, 2025

Copy link
Copy Markdown
Member

Now merged upstream, so I suppose we can simply revert this now. ^_^

@jacopo-degattis

jacopo-degattis commented Jul 15, 2025

Copy link
Copy Markdown
Author

Yes I confirm it's been merged upstream.
We can revert and remove the previous patch then ! ;)

@poire-z poire-z mentioned this pull request Jul 19, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Frenzie Frenzie Frenzie left review comments
@NiLuJe NiLuJe NiLuJe approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jacopo-degattis @NiLuJe @Frenzie