Opened 14 months ago

Closed 8 months ago

#27947 closed defect (fixed)

Chutney's owning controller process code compares strings with ints

Reported by: teor Owned by: teor
Priority: Medium Milestone:
Component: Core Tor/Chutney Version:
Severity: Normal Keywords: fast-fix
Cc: Actual Points: 0.1
Parent ID: Points: 0.1
Reviewer: nickm Sponsor: Sponsor19

Description

Chutney's owning controller process code compares an environmental variable with an int, without converting the string to an int.

Child Tickets

Change History (11)

comment:1 Changed 14 months ago by teor

Reported by Steven Engler, who pointed us to these lines:
https://github.com/torproject/chutney/blob/master/lib/chutney/TorNet.py#L877-L879

comment:2 Changed 14 months ago by dgoulet

Milestone: Tor: unspecified

comment:3 Changed 14 months ago by teor

Milestone: Tor: unspecified

comment:4 Changed 14 months ago by opara

Would you be okay with a patch that moves these lines out of the TorNet.py file and into the test-network.sh script? It would make more sense to me to keep these out of TorNet.py since these three environment variables don't seem relevant to Chutney as a whole, but more just for the test network script.

On the other hand there may be reasons why they were put in TorNet.py, and in that case, I can submit a patch which fixes the problem directly (wraps the os.environ.get() in an int() call).

comment:5 in reply to:  4 Changed 14 months ago by teor

Replying to opara:

Would you be okay with a patch that moves these lines out of the TorNet.py file and into the test-network.sh script? It would make more sense to me to keep these out of TorNet.py since these three environment variables don't seem relevant to Chutney as a whole, but more just for the test network script.

On the other hand there may be reasons why they were put in TorNet.py, and in that case, I can submit a patch which fixes the problem directly (wraps the os.environ.get() in an int() call).

When CHUTNEY_CONTROLLING_PID is set, chutney sets the __OwningControllerProcess torrc option, so that tor exits when chutney is finished. But when any of these environmental variable are negative numbers, the user wants tor to keep running after chutney exits, so we don't set __OwningControllerProcess.

So we need to patch TorNet.py to cast the strings to ints.

Edit: typo

Last edited 14 months ago by teor (previous) (diff)

comment:6 Changed 12 months ago by nickm

Component: Core Tor/TorCore Tor/Chutney

comment:7 Changed 12 months ago by teor

opara, did you still want to write this patch?

comment:8 Changed 9 months ago by teor

Keywords: fast-fix added

comment:9 Changed 8 months ago by teor

Actual Points: 0.1
Points: 0.1
Reviewer: nickm
Sponsor: Sponsor19

We need this bug fixed to test PTs and other chutney tests.

Here's a pull request:
https://github.com/torproject/chutney/pull/15

comment:10 Changed 8 months ago by nickm

Status: assignedmerge_ready

LGTM, and tests pass. Feel free to merge!

comment:11 Changed 8 months ago by teor

Resolution: fixed
Status: merge_readyclosed

Merged to master as ce93bf0

Note: See TracTickets for help on using tickets.