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

Breaks with Special characters or spaces in d.name with rtcontrol #5

Open
Inevitable opened this issue Dec 3, 2024 · 10 comments
Open

Comments

@Inevitable
Copy link

Been using Q4D in it's older form for quite some time, so thanks for creating it. I'm trying to configure this new version, as it seems like the prior iteration/v1 is no longer available. I'll note that with the original implementation, it did not seem to have any of the issues I cover in this post.

I've also moved to pyrosimple to provide rtcontrol since classic pyrocore is still python2 bound, and dealing with working around that deprecation is annoying.

Mostly functions as expected, except in cases where the Name contains special characters. I believe it also has issues with spaces, but haven't been able to conclusively prove that as a failure condition, just infer based on output.

As an example, using the RTCONTROL method, a torrent with a Name attribute structurally like

[group] Some Title of content - 10 (SOME ATTRIBUTES OF FILE).extension

Seems to be parsed as the following, which I'm guessing is just playing all hell with the payloadDetails array stuff

?group]Some: Title: of: content-: 10(SOME: ATTRIBUTESOF: FILE).extension

At least I assume, this is just from me throwing a quick printf ${_rtcFile} at the end of the RTCONTROL payloadDetails stuff.
Interesting aspects of note: it seems like [ causes particular issues, the insertion of the : characters, and the weird removal of some but not all spaces.

The standard log entry produced by an unmodified Queue4Download.sh has the Name correct, but the other fields are missing/broken:

<SUCCESS> [group] Some Title of content - 10 (SOME ATTRIBUTES OF FILE).extension ( ) ( 1 ) [0 secs]
Transfer <44> ( QUEUED )

The mqtt events are also, as expected, not correct. Down doesn't get a message, Label only gets:

QUEUED	

No hash, as, I assume rtcontrol is failing to return anything if the filter passed to it is ?group]Some: Title: of: content-: 10(SOME: ATTRIBUTESOF: FILE).extension

This of course also means that it is neither labeled nor downloaded!

Seems like perhaps we need to handle (d.name) differently so that it is passed to the script properly escaped/quoted to prevent the parsing issues. We'd also then need to deal with the mqtt event and client side, since it seems to fall over when handed events where the name is double or single quoted...

Okay, sorry for the stream of consciousness/wall of text. Any ideas on how to remediate/fix?

@weaselBuddha
Copy link
Owner

Weird. Queue4Download.sh deals with odd character embedded names without a problem on my end. That would point towards pyrosimple.

I need to spin up a rtorrent instance and take a look.

@Inevitable
Copy link
Author

A good bet then, I'll give their -legacy branch a shot which supposedly is a 1:1 for pyrocore. Apparently the default pyrosimple distro includes some enhancements.

If not, then I guess I get to try and figure out how to cram python2 into this alpine container (was hard deprecated in like 3.12).

@weaselBuddha
Copy link
Owner

apk add -U python2 ?

@Inevitable
Copy link
Author

Inevitable commented Dec 3, 2024

Nah, pkg is just straight up gone. 3.15 from 2021 is the final version in the community repo. was gone from main in 3.12

I could do something extremely jank in build like

echo "@315 http://dl-cdn.alpinelinux.org/alpine/v3.15/community/" >> /etc/apk/repositories \
  && apk --update --no-cache add python2@315

But would prefer to just not run software that's been deprecated and unmaintained for 5 years

@Inevitable
Copy link
Author

Inevitable commented Dec 3, 2024

Tested with the release-1.X branch of pyrosimple, doing a simple rtcontrol name="torrent_name" -o "hash" Works with an item that has a sanitized name debian-12.8.0-amd64-netinst.iso but fails instantly with something formatted like the example in the initial issue...

Damn, I really might have to see if I can shoehorn python2 on there, which is extra annoying since 3 is needed for some other stuff

@Inevitable
Copy link
Author

Yeah, gave the python2 thing a go, but it's just a mess of overlapping masked deps and pain. Would have to basically re-base the whole stack on alpine 3.14 which is just a no go.

Is there a way to see the output being passed by rtorrent when it calls the Queue script? I don't think it's exotic/strange, but would like to have a way to see it so I can try maybe escaping the output somehow so it works with pyrosimple...

I notice in this new version, it's using a slightly different method (d.name) vs the prior which I think was $d.name= haven't the faintest idea if there is any difference. Tried the older, didn't seem to make a difference, so I assume they're equivalent?

@weaselBuddha
Copy link
Owner

You can run Queue4Download.sh from the command line with the d.name, bash has two parameters to assist in debugging:

bash -xv ~/.Q4D/Queue4Download.sh "[group] Some Title of content - 10 (SOME ATTRIBUTES OF FILE).extension"

Additionally, there is a logfile already defined, you can add echoes if that helps.

Q4Ddefines.sh:readonly SERVER_LOGFILE=$Q4D_PATH/queue.log )

You can try replacing

RTCONTROL can also be a wrapper, it is

Q4Ddefines.sh:readonly _RTCONTROL=~/bin/rtcontrol

Changing that as say: ~/.Q4D/rtcontrol_wrapper.sh will let you investigate parameters

The reason d.name is used is to make it easy to run from the command line, there is no requirement that it be that. You can use d.hash and look up the other fields.

The name is just a convenience (logging, invoking) path (for LFTP) and hash (for labeling) are the key field. Typing if using also can use name and label in assigning a type code but not required.

'=' is just a terminator in the .rtorrent.rc parlance, it isn't necessary

This is what I had in my .rtorrent.rc

method.set_key = event.download.finished,complete,"execute.throw.bg=/home/owner/.Q4D/Queue4Download.sh,(d.name)"

I still need to spin up an rtorrent instance.

@Inevitable
Copy link
Author

Think I managed to bludgeon my way to success mostly. Swapping to hash for the lookups made a big difference (I just swapped KEY and HASH in the line 31 section, essentially). I also had to add some quote escaping where payloadDetails[KEY] and payloadDetails[PATH] were used, though I'm pretty certain I went overboard there since I just applied to all instances and then trial-and-error'd until it worked.

payloadDetails[KEY]="\"${payloadDetails[PATH]}\"" (line 117) as an example.
Similar touches to 187, 203

I'm almost certain there is a better/cleaner/more correct way to do this, maybe moving to a more structured mqtt event rather than plaintext, but so far it seems functional.

@weaselBuddha
Copy link
Owner

Glad you've made progress.

There is a embedded function in BASH you can use "quote". so for example:

  payloadDetails[KEY]=$(quote "${payloadDetails[PATH]}")

Mosquitto mqtt does support JSON. I chose to use plaintext to adhere to the KISS principle, introducing encoding/decoding logic also introduced complexity, and increase the probability of bugs and reduced it's adaptability. Idea through-out was to make it all CLI readable so it could be looked at and used/debugged/refactored from the command line.

I'm in process in setting up an rtorrent instance, but my suspicion here is that this is a "compatible with new versions" issue. I gave up my rtorrent about a month ago, I've been running Q4D for several years. The new version since last year. New version was motivated by easing adoption (config files and alike) and inclusion of other torrent clients outside of rtorrent,

@SteveGilvarry
Copy link

FYI filter in single quotes works
rtcontrol 'name="torrent space name"' -o "hash"
just haven't managed to get that working from script yet, without the double quotes resulting in rtcontrol treating string as a series of names. I suspect with this you don't need to replace special characters.

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

No branches or pull requests

3 participants