Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.2
    • Fix Version/s: None
    • Component/s: Database Core
    • Labels:
    • Skill Level:
      Regular Contributors Level (Easy to Medium)

      Description

      Today I received a bug report from a user who thought CouchDB might be relaxing and evaluate an arithmetic expression in the config. (That makes sense, since it seems to evaluate erlang terms elsewhere.)

      https://getsatisfaction.com/iriscouch/topics/my_couchdb_is_broken_after_putting_a_badarg_for_a_configuration_value

      It would be nice for CouchDB to validate config input, particularly when bad values permanently take it offline. (In this case, it returns 500 for all subsequent requests.)

      IMO, a good bang-for-buck is to have three basic value types:

      1. String
      2. Erlang tuple
      3. Integer

      And each setting knows what type it must be.

        Issue Links

          Activity

          Hide
          Paul Joseph Davis added a comment -

          I've seen similar reports but nothing scary enough to worry too much. Though your comment sparks an idea:

          What if all config values are specified as Erlang terms, perhaps even so much as going to a full on Erlang term file format and then for each setting that's available we create a function that does input validation. Also, similarly we can store help information along with the function that does validation and then use that in building docs (we have a similar thing in Gunicorn that seems to work well enough).

          Also, this would allow apps to register functions at boot to allow the system to be extensible. Settings that don't have a validator are stored but not used (or rather, if they they shouldn't cause failures, etc). Only thing I see is what to do if you register a handler for a value that exists but is invalid. Perhaps replace it?

          Anyway, good idea, just not sure on the details (ie, tuples only? what about tuples that contain terms? Or our handful that are lists?)

          Show
          Paul Joseph Davis added a comment - I've seen similar reports but nothing scary enough to worry too much. Though your comment sparks an idea: What if all config values are specified as Erlang terms, perhaps even so much as going to a full on Erlang term file format and then for each setting that's available we create a function that does input validation. Also, similarly we can store help information along with the function that does validation and then use that in building docs (we have a similar thing in Gunicorn that seems to work well enough). Also, this would allow apps to register functions at boot to allow the system to be extensible. Settings that don't have a validator are stored but not used (or rather, if they they shouldn't cause failures, etc). Only thing I see is what to do if you register a handler for a value that exists but is invalid. Perhaps replace it? Anyway, good idea, just not sure on the details (ie, tuples only? what about tuples that contain terms? Or our handful that are lists?)
          Hide
          Paul Joseph Davis added a comment -

          Also, while I do agree this is a good idea, the setting that borked is a bug with how that value is used when it's found. Regardless of how strict you are on type information it'll always be possible to throw a wrench in the gears with an unexpected value in the config. But having some sort of internal tooling to handle the parsing and presentation of these values that is separate from the code where the value is used seems like it'd be quite beneficial from an engineering perspective.

          Show
          Paul Joseph Davis added a comment - Also, while I do agree this is a good idea, the setting that borked is a bug with how that value is used when it's found. Regardless of how strict you are on type information it'll always be possible to throw a wrench in the gears with an unexpected value in the config. But having some sort of internal tooling to handle the parsing and presentation of these values that is separate from the code where the value is used seems like it'd be quite beneficial from an engineering perspective.
          Hide
          Jason Smith added a comment -

          That is a pretty exciting idea. I find two things very appealing:

          1. Belt-and-suspenders: validate the input as much as reasonable, but also don't use bad data if it's in there (because a user could always edit the .ini; or just generally speaking, one subsystem shouldn't trust the other).
          2. Extend the config system to be more general and more useful.

          However IMO this ticket addresses a pretty restricted scope: input validation in the 1.2.x codebase. Normally I wouldn't nitpick except that this issue came from a real-world user not particularly steeped in CouchDB culture or community. It has street cred.

          Can we get a conversation about the broader feature set in CouchDB 3000, and let that inform a more modest quick-fix in the 1.2.x code line?

          Show
          Jason Smith added a comment - That is a pretty exciting idea. I find two things very appealing: 1. Belt-and-suspenders: validate the input as much as reasonable, but also don't use bad data if it's in there (because a user could always edit the .ini; or just generally speaking, one subsystem shouldn't trust the other). 2. Extend the config system to be more general and more useful. However IMO this ticket addresses a pretty restricted scope: input validation in the 1.2.x codebase. Normally I wouldn't nitpick except that this issue came from a real-world user not particularly steeped in CouchDB culture or community. It has street cred. Can we get a conversation about the broader feature set in CouchDB 3000, and let that inform a more modest quick-fix in the 1.2.x code line?
          Hide
          Paul Joseph Davis added a comment -

          I understand your concern, but (at least at this hour after a few beers) I'm not seeing much of a distinction. A "validate all the inputs" doesn't seem far off the (IMO simpler) reorganization to Erlang term format.

          IMO, the immediate solution to this patch is fixing the bug the user encounter at the source where the config value is used and the fancier validation split off as its own thing.

          Show
          Paul Joseph Davis added a comment - I understand your concern, but (at least at this hour after a few beers) I'm not seeing much of a distinction. A "validate all the inputs" doesn't seem far off the (IMO simpler) reorganization to Erlang term format. IMO, the immediate solution to this patch is fixing the bug the user encounter at the source where the config value is used and the fancier validation split off as its own thing.
          Hide
          Alexander Shorin added a comment -

          Just to let me not forget about, while there is special ticket about config settings validation...

          ...trivial case when someone have tried to rename system database (e.g. from _user to _my_users - why not? is this database systems or not?) makes CouchDB instance fall with same symptoms. So I suppose config checks should not be only "about value type", but also "about value data" for some parameters.

          Show
          Alexander Shorin added a comment - Just to let me not forget about, while there is special ticket about config settings validation... ...trivial case when someone have tried to rename system database (e.g. from _user to _my_users - why not? is this database systems or not?) makes CouchDB instance fall with same symptoms. So I suppose config checks should not be only "about value type", but also "about value data" for some parameters.

            People

            • Assignee:
              Jason Smith
              Reporter:
              Jason Smith
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:

                Development