Validate database prefix against DBNAME_REGEX for system dbs #1647
Conversation
|
hello, i just want to make sure that this doesn't break our current usage. |
|
It would still work: |
iilyak
force-pushed
the
validate-prefix-for-systemdbs
branch
3 times, most recently
from
eedd250
to
695b633
Compare
|
src/couch/src/couch_db.erl
Outdated
| {Prefix, true} -> | ||
| re:run(Prefix, ?DBNAME_REGEX, [{capture,none}, dollar_endonly]) | ||
| == | ||
| match |
There was a problem hiding this comment.
Not sure why == match are not on the same line, but I guess it's not a big deal.
There was a problem hiding this comment.
I wish we (CouchDB community) have a syntax guidelines. I agree, this formatting is a bit haskellish (or ocamellish). However it provides a good structure visually which helps when reading the code. Although it could be just me because I used to this formatting. Let me know if you want me to change it and which one you would prefer.
- operand (new line) operator (new line) operand
re:run(Prefix, ?DBNAME_REGEX, [{capture,none}, dollar_endonly])
==
match
- operand (new line) (extra indent) operator operand
re:run(Prefix, ?DBNAME_REGEX, [{capture,none}, dollar_endonly])
== match
- operand (new line) operator operand
re:run(Prefix, ?DBNAME_REGEX, [{capture,none}, dollar_endonly])
== match
- extract this into a function
match_regexp/1and use it in other places as well.
There was a problem hiding this comment.
personally I'd split it i two lines
ReOpts = [{capture,none}, dollar_endonly]
re:run(Prefix, ?DBNAME_REGEX, ReOpts) == match
as it is right now looks more markupish than haskellish to me.
Previously we only checked that the suffix of the database is matching one of the predefined system databases. We really should check the prefix against DBNAME_REGEXP to prevent creation of illegally named databases. This fixes apache#1644
iilyak
force-pushed
the
validate-prefix-for-systemdbs
branch
from
695b633
to
aa63804
Compare
| Suffix = filename:basename(Normalized), | ||
| case {filename:dirname(Normalized), lists:member(Suffix, ?SYSTEM_DATABASES)} of | ||
| {<<".">>, Result} -> Result; | ||
| {Prefix, false} -> false; |
There was a problem hiding this comment.
Just happen to find warning .../db/src/couchdb/src/couch/src/couch_db.erl:1748: Warning: variable 'Prefix' is unused
Overview
Previously we only checked that the suffix of the database is
matching one of the predefined system databases. We really should
check the prefix against DBNAME_REGEXP to prevent creation of
illegally named databases.
Testing recommendations
Related Issues or Pull Requests
This fixes #1644
Checklist