-
Notifications
You must be signed in to change notification settings - Fork 30
allow more characters in cookie #106
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
Conversation
big-r81
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
nickva
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with:
$ export cookie=a:/b@1x
$ cat text.txt
# -setcookie foo
$ sed -i "$(printf 's\30^[# ]*-setcookie.*$\30-setcookie %s\30' ${cookie})" text.txt
$ cat text.txt
-setcookie a:/b@1x
|
The rpm side of things could have the same problem: couchdb-pkg/rpm/SPECS/couchdb.spec.in Line 156 in c928bd6
Should we fix it there too, or as a separate PR perhaps? |
3bc1118 to
945f928
Compare
|
Setting the cookie worked. I rebuilt debian-buster deb with it and installed in a VM. deb install dialog: vm.args file CouchDB seems to start and I can access it via curl. However, unfortunately the single quotes seem to break our remsh script. Likely due to assumptions how we parse the cookie value there https://github.com/apache/couchdb/blob/3181d928e060687e2a214192ba17c401811c6da3/rel/overlay/bin/remsh#L50-L57 |
|
you can specify a cookie with spaces if you do so correctly; Just the same as you would have to single quote that atom in erlang code itself. My modification at least allows the use of I am not sure how to do the same on the RPM side as it appears not to be a shell script as such. I will try, however. |
e24a362 to
567d1c4
Compare
|
Testing with commit 567d1c4 on a Debian Buster VM: I see cookie: another example cookie: RPM test on CentOS 7 It seems the replacement had stopped working both for the user supplied cookie in a variable or for the randomly generated one: For random generated one: |
aa379cf to
ff6e59f
Compare
A cookie value with a '/' in it caused a sed error during postinst; ``` sed: -e expression #1, char 53: unknown option to `s' ``` We use a control character (RS - record separator) instead of / to reduce the chances of a collision with a valid cookie string. I applied the same change to nodename even though a / in node name would be an error, for consistency.
ff6e59f to
1b18b7c
Compare
|
Testing with 1b18b7c RPM on CentOS 7 works as expected: Random new cookie: Custom cookie: remsh didn't work but that's expected, I re-wrote bin/remsh to use a similar vm.args file with just the Works on Debian Buster Install dialog: We'll have to fix remsh cookie parsing in the main repo before the next release. But cookie prompt and replacement in this repo works well. Thanks for the fix! +1 |
A cookie value with a '/' in it caused a sed error during postinst;
Instead of a sed substitution we use
/cto replace the cookie line. this permits most characters to get through. we escape backslashes prior to interpreting them when constructing the sed line.I applied the same change to nodename even though a / in node name would be an error, for consistency.