Novik / ruTorrent

Yet another web front-end for rTorrent
Other
2.03k stars 414 forks source link

_getdir regression with whitespace and nbsp #2779

Closed anthonyryan1 closed 1 week ago

anthonyryan1 commented 2 weeks ago

What would you like changed about the web client?

Commit https://github.com/Novik/ruTorrent/commit/38c6489e60729fe841a5e6904ea288fd74e1f8fe / https://github.com/Novik/ruTorrent/pull/2768 appears to have introduced a regression with whitespace handling in 5.1 beta 4.

Since this commit, /plugins/create/action.php cmd: "create" is getting unicode non-breaking spaces xC2A0, instead ASCII spaces and throwing errors during torrent creation, when the file name creates a torrent. This is causing non-existing path errors.

Reproduction:

  1. Create a file with spaces in the name dd if=/dev/zero bs=1024 count=1 of="file with spaces.bin"
  2. Open Dev tools network tab, and filter by "plugins/create/action.php"
  3. Select "file with spaces.bin" as your source in the "Create New Torrent" dialog.
  4. Create...
  5. Copy path_edit from the request arguments and convert to hex to observe that the spaces are not ASCII.

Describe the solution you'd like and propose possible alternatives.

It looks like part of that change is juggling between \u00a0 (equal to hex xC2A0) and ascii whitespace. I expect the intention there was to tackle line wraps in the directory selector menu?

When the create plugin is getting the value out, it's using jQuery's .val() without any fancy substitution. as seen here: https://github.com/Novik/ruTorrent/blob/v5.1-beta4/plugins/create/init.js#L19

I don't fully understand the initial motivation behind the non breaking spaces, but my instinct is that if the nbsp replacements were used for layout purposes, there's probably a CSS trait we could use instead and keep the proper ASCII spaces in the code and avoid juggling between valid strings, and invalid-but-displayed strings.

Provide information and resources about the environment hosting the web client.

ruTorrent: 5.1_beta4 Browser: Firefox 132.0.1 PHP: 8.3.13 OS: Gentoo Linux (rolling release) Install script: Roughly https://gitweb.gentoo.org/repo/gentoo.git/tree/www-apps/rutorrent/rutorrent-4.3.9.ebuild

Create torrent backend is mktorrent.sh / mktorrent 2021-01-30

Additional context

No response

anthonyryan1 commented 2 weeks ago

CC @jevenski for information about the motivation behind the nbsp conversions.

jevenski commented 2 weeks ago

The plugin had problems displaying file names that contain white spaces in them. The solution was borrowed from filemanager plugin. Looks like we'll have to further look into this issue.

Ref: https://github.com/Novik/ruTorrent/issues/2757

anthonyryan1 commented 2 weeks ago

That should be solvable with

.rmenuitem {
    white-space: pre;
}

Which will make us treat all whitespace as significant without converting.

I'll ready a pull request.

anthonyryan1 commented 2 weeks ago

The other half of this issue (the non-nbsp) parts all seem collectively responsible for breaking single-file torrent creatino for files with spaces in the name.

I'm not familiar enough with the trailing slash problem, but I think that the rest of it will need to be revisited as well.

Do you have any input on that part @jevenski ? I've ended up reverting the rest of that commit as well now for my deployments.

My new pull request fixes the nbsp problems, and the directory with spaces problem. But not creation of a single file torrent with "spaces in the name.bin"

jevenski commented 2 weeks ago

The old solution only replaced Unicode non-breaking spaces (0x00a0) back to ASCII spaces when submitting a request for the new directory list. The torrent creation script still received Unicode non-breaking spaces, as well as any other backend scripts did when receiving path parameter.

I think reverting the substitution between Unicode spaces and ASCII spaces, and applying white-space: pre; to display multiple white spaces correctly is the right way to go. I've tried this solution. The create plugin can now create torrents with a single file that has white spaces in the file name ("spaces in the name.bin").

anthonyryan1 commented 1 week ago

Re-opening this issue. The pull request fixes torrent creation with directories containing spaces (and the file not existing errors).

But we still need to land a fix for torrent files with spaces in their names.

jevenski commented 1 week ago

I tried both internal ($useExternal = false) and mktorrent. Both of the two methods seem to handle well both file names and directory names with spaces in them.

stickz commented 1 week ago

Thanks @jevenski one more issue left before posting Beta 5.