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

Add trailing nonce to null-flushed pipes (fixes #17) #171

Closed
wants to merge 1 commit into from

Conversation

TinoDidriksen
Copy link
Member

Works around issue #17 by adding a trailing nonce. The nonce is cut off from the text by nulls on both sides, just in case. Post-translation all nulls are removed, in case any extras got output by either previous or this run.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 47.764% when pulling f45d458 on nonce into 151e3e2 on master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 47.764% when pulling f45d458 on nonce into 151e3e2 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 47.764% when pulling f45d458 on nonce into 151e3e2 on master.

@unhammer
Copy link
Member

unhammer commented Nov 5, 2020

Is there any chance the nonce could be eaten by a pipeline? In that case, the pipeline will hang and further requests are blocked. I tested this by making a script

#!/bin/bash
i=0
while read -r -d '' line; do
    if [[ $(( ++i )) -gt 3 ]]; then
        messedup=${line//NONCE/}
        printf "%s\0" "${messedup}"
    else
        printf "%s\0" "${line}"
    fi
done

and putting it in the middle of a pipeline and running servlet and curling against my modified pipeline – the second request then just hangs, and all further requests to that pipeline also hang.

This was probably already a problem c.f. comment

# TODO: If the output has no \0, this hangs, locking the
# pipeline. If there's no way to put a timeout right here, we
# might need a timeout using Pipeline.use(), like servlet.py's
# cleanable but called *before* trying to translate anew

(but pipelines tended to produce quite a lot of NUL's so instead of hanging they gave the wrong output?).

I added a timeout that seems to fix it here:

https://github.com/apertium/apertium-apy/compare/nonce...pr-171?expand=1

@TinoDidriksen
Copy link
Member Author

If pipes start eating or modifying superblanks, we have larger issues. But yes, a timeout is great - I just didn't want to mess with more Python than absolute minimum.

@unhammer
Copy link
Member

unhammer commented Nov 5, 2020

rebased and merged as 4188445 :)

@unhammer unhammer closed this Nov 5, 2020
@TinoDidriksen TinoDidriksen deleted the nonce branch November 5, 2020 14:01
@TinoDidriksen TinoDidriksen restored the nonce branch April 1, 2021 09:50
@TinoDidriksen TinoDidriksen deleted the nonce branch April 1, 2021 09:50
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

Successfully merging this pull request may close these issues.

None yet

3 participants