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

Handle slashes in names #9

Merged
merged 5 commits into from
Mar 15, 2017

Conversation

ryanjyoder
Copy link
Contributor

Slashes in DB names should be escaped. The same is true for document ids.

I used PathEscape (in net/url) which is only available in go 1.8.

Also note, there is a known bug with couchdb not handling literal '+' in database names (and document ids). I'm not testing for this as it fails on couchdb's end.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.192% when pulling 8bc6aab on ryanjyoder:handle_slashes_in_names into 6b4d7ca on zemirco:master.

@zemirco
Copy link
Owner

zemirco commented Mar 13, 2017

Thank you! Will take a look within the next days.

Copy link
Owner

@zemirco zemirco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor things. Looks great overall!

client_test.go Outdated
// The folllowing characters are valid in databse names: /_$()-+
// However there is a known bug in couchdb with + (skipping + for testing)
// https://issues.apache.org/jira/browse/COUCHDB-1580
validSpecialCharacter = "/_$()-/"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the trailing slash /? Isn't this character already tested with the first slash?

And is that documented somewhere which characters are allowed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we can remove the second slash.
http://docs.couchdb.org/en/1.6.0/api/database/common.html

The database name {db} must be composed by following next rules:
Name must begin with a lowercase letter (a-z)
Lowercase characters (a-z)
Digits (0-9)
Any of the characters _, $, (, ), +, -, and /.

If you’re familiar with Regular Expressions, the rules above could be written as ^[a-z][a-z0-9_$()+/-]*$.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For document id, I haven't been able to find any restrictions however...

client_test.go Outdated
@@ -17,6 +17,14 @@ import (

var client *Client

const (
// The folllowing characters are valid in databse names: /_$()-+
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in databse.

client_test.go Outdated
@@ -17,6 +17,14 @@ import (

var client *Client

const (
// The folllowing characters are valid in databse names: /_$()-+
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in folllowing.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 71.192% when pulling b63272f on ryanjyoder:handle_slashes_in_names into 6b4d7ca on zemirco:master.

@zemirco zemirco merged commit 68d2a24 into zemirco:master Mar 15, 2017
@ryanjyoder ryanjyoder deleted the handle_slashes_in_names branch March 30, 2017 04:22
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