Mercurial > p > roundup > code
comparison roundup/rest.py @ 6554:576d630fc908
Fix error status for invalid props
Some places were raising AttributeError which results in a 405 (bad
method) not a 400. Replace with UsageError or KeyError.
Make rest.py::transitive_props run aginst a prop that has no
transitive elements as well. So it will verify that assignedto exists
even though it has no period like assignedto.name would.
These should check properties in @fields and @sort.
Also validate fields that are used as search params:
?assignedto=1
If the search prop was mispelled or incorrect, the search element was
ignored as though it had not been specified. Now it returns a 400 to
notify sender they sent an incorrect filter.
Also remove unused statements that were originally for finding invalid
props before we supported transitive props.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Sat, 11 Dec 2021 21:41:49 -0500 |
| parents | 9aa8df0b4426 |
| children | 178705fbeaa8 |
comparison
equal
deleted
inserted
replaced
| 6553:75da037d1c54 | 6554:576d630fc908 |
|---|---|
| 561 if '.' in p: | 561 if '.' in p: |
| 562 prop = None | 562 prop = None |
| 563 for pn in p.split('.'): | 563 for pn in p.split('.'): |
| 564 # Tried to dereference a non-Link property | 564 # Tried to dereference a non-Link property |
| 565 if cn is None: | 565 if cn is None: |
| 566 raise AttributeError("Unknown: %s" % p) | 566 raise UsageError("Property %(base)s can not be dereferenced in %(p)s." % { "base": p[:-(len(pn)+1)], "p": p}) |
| 567 cls = self.db.getclass(cn) | 567 cls = self.db.getclass(cn) |
| 568 # This raises a KeyError for unknown prop: | 568 # This raises a KeyError for unknown prop: |
| 569 try: | 569 try: |
| 570 prop = cls.getprops(protected=True)[pn] | 570 prop = cls.getprops(protected=True)[pn] |
| 571 except KeyError: | 571 except KeyError: |
| 572 raise AttributeError("Unknown: %s" % p) | 572 raise KeyError("Unknown property: %s" % p) |
| 573 if isinstance(prop, hyperdb.Multilink): | 573 if isinstance(prop, hyperdb.Multilink): |
| 574 raise UsageError( | 574 raise UsageError( |
| 575 'Multilink Traversal not allowed: %s' % p) | 575 'Multilink Traversal not allowed: %s' % p) |
| 576 # Now we have the classname in cn and the prop name in pn. | 576 # Now we have the classname in cn and the prop name in pn. |
| 577 if not self.db.security.hasPermission('View', uid, cn, pn): | 577 if not self.db.security.hasPermission('View', uid, cn, pn): |
| 580 % (cn, pn))) | 580 % (cn, pn))) |
| 581 try: | 581 try: |
| 582 cn = prop.classname | 582 cn = prop.classname |
| 583 except AttributeError: | 583 except AttributeError: |
| 584 cn = None | 584 cn = None |
| 585 else: | |
| 586 cls = self.db.getclass(cn) | |
| 587 # This raises a KeyError for unknown prop: | |
| 588 try: | |
| 589 prop = cls.getprops(protected=True)[pn] | |
| 590 except KeyError: | |
| 591 raise KeyError("Unknown property: %s" % pn) | |
| 585 checked_props.append (p) | 592 checked_props.append (p) |
| 586 return checked_props | 593 return checked_props |
| 587 | 594 |
| 588 def error_obj(self, status, msg, source=None): | 595 def error_obj(self, status, msg, source=None): |
| 589 """Return an error object""" | 596 """Return an error object""" |
| 800 verbose = int(value) | 807 verbose = int(value) |
| 801 elif key == "@fields" or key == "@attrs": | 808 elif key == "@fields" or key == "@attrs": |
| 802 f = value.split(",") | 809 f = value.split(",") |
| 803 if len(f) == 1: | 810 if len(f) == 1: |
| 804 f = value.split(":") | 811 f = value.split(":") |
| 805 allprops = class_obj.getprops(protected=True) | |
| 806 display_props.update(self.transitive_props(class_name, f)) | 812 display_props.update(self.transitive_props(class_name, f)) |
| 807 elif key == "@sort": | 813 elif key == "@sort": |
| 808 f = value.split(",") | 814 f = value.split(",") |
| 809 allprops = class_obj.getprops(protected=True) | |
| 810 for p in f: | 815 for p in f: |
| 811 if not p: | 816 if not p: |
| 812 raise UsageError("Empty property " | 817 raise UsageError("Empty property " |
| 813 "for class %s." % (class_name)) | 818 "for class %s." % (class_name)) |
| 814 if p[0] in ('-', '+'): | 819 if p[0] in ('-', '+'): |
| 843 try: | 848 try: |
| 844 prop = class_obj.getprops()[p] | 849 prop = class_obj.getprops()[p] |
| 845 except KeyError: | 850 except KeyError: |
| 846 raise UsageError("Field %s is not valid for %s class." % | 851 raise UsageError("Field %s is not valid for %s class." % |
| 847 (p, class_name)) | 852 (p, class_name)) |
| 853 # Call this for the side effect of validating the key | |
| 854 _ = self.transitive_props(class_name, [ key ]) | |
| 848 # We drop properties without search permission silently | 855 # We drop properties without search permission silently |
| 849 # This reflects the current behavior of other roundup | 856 # This reflects the current behavior of other roundup |
| 850 # interfaces | 857 # interfaces |
| 851 # Note that hasSearchPermission already returns 0 for | 858 # Note that hasSearchPermission already returns 0 for |
| 852 # non-existing properties. | 859 # non-existing properties. |
| 1022 props = set() | 1029 props = set() |
| 1023 # support , or : separated elements | 1030 # support , or : separated elements |
| 1024 f = value.split(",") | 1031 f = value.split(",") |
| 1025 if len(f) == 1: | 1032 if len(f) == 1: |
| 1026 f = value.split(":") | 1033 f = value.split(":") |
| 1027 allprops = class_obj.getprops(protected=True) | |
| 1028 props.update(self.transitive_props(class_name, f)) | 1034 props.update(self.transitive_props(class_name, f)) |
| 1029 elif key == "@protected": | 1035 elif key == "@protected": |
| 1030 # allow client to request read only | 1036 # allow client to request read only |
| 1031 # properties like creator, activity etc. | 1037 # properties like creator, activity etc. |
| 1032 # used only if no @fields/@attrs | 1038 # used only if no @fields/@attrs |