Fork me on GitHub
#yada
<
2016-07-21
>
martinklepsch06:07:48

In Bidi: Is it intentional that ["plan/" :id] matches plan/x but not plan/x:x?

martinklepsch07:07:17

It's easily fixable by providing a matching pattern [#"\w+:\w+" :id] but it seems that it should match by default?

malcolmsparks07:07:50

@martinklepsch: by default, the keyword regex is quite strict to defend against injection attacks. For example, a : has a specific meaning on Windows filenames and could trigger a vulnerability. By choosing your own regex you are choosing your own defences.

martinklepsch07:07:17

Ok, that makes sense. I just saw that it is valid according to spec so I expected bidi to accept it as well

malcolmsparks07:07:32

Yes, lots of strings are valid in a URL such as "100 ; DELETE DATABASE" etc.

malcolmsparks07:07:11

Not all vulnerabilities are caused by spec violations so I think implementations should steer towards secure defaults

martinklepsch07:07:35

y, with that in mind I'm totally on your side

malcolmsparks07:07:16

@ijbriscoe: I just checked the source in yada.methods. If you want a 201 from a POST you need to pass back a java.net.URI of the location of the new resource. The relevant spec here is RFC 7231 section 4.3.3 - a 201 response must contain a Location header. Yada cannot infer it

malcolmsparks07:07:09

If you need more control, return (:response ctx), or a modification of it

lmergen08:07:03

the same is true for 202, btw

lmergen08:07:08

for 201 Created, the location header should point to the URI of the resource that was created, for 202 Accepted, the location header should point to the URI where the client can check the status of the request

lmergen08:07:24

(i was not aware of this, so TIL)

malcolmsparks08:07:52

Yes. The exact behavior is in yada.methods. If you don't like my decisions there you can provide your own. You can extend yada and create new methods this way too.

malcolmsparks08:07:09

I'm trying to strike the right balance between user control and correctness (wrt specifications). This isn't always easy. Ring goes for

malcolmsparks08:07:16

a preference of control over correctness. The point of adopting yada is to improve correctness and thereby create a better web

malcolmsparks08:07:57

So reach for escape hatches as a last resort

malcolmsparks08:07:30

I hope over time yada will get better and these issues will get easier

ijbriscoe10:07:40

@malcolmsparks: Thanks for the explanation. I'd forgotten about the need for a Location header, which illustrates why it's a good thing for yada to take care of things like this! Given it can't infer the URL, returning it seems like a good approach.

ijbriscoe10:07:44

That said - and I've just looked at the Edge example of a POST method - this doesn't work too well for me, as I would prefer to return a body containing the result of saving the document to ArangoDB, combined with the original payload...

ijbriscoe10:07:31

So, if we're being correct, would there usually be no body from a POST, and if you want a full copy of the resource, you'd have to make a GET request using the URL in the Location header to get it?

malcolmsparks10:07:01

> The action performed by the POST method might not result in a resource that can be identified by a URI. In this case, either 200 (OK) or 204 (No Content) is the appropriate response status, depending on whether or not the response includes an entity that describes the result.

malcolmsparks10:07:02

So the question is then, how to return both the location and a body? In this case you need to return a modified (:response ctx) , but perhaps there ought to be a better approach

malcolmsparks10:07:46

Also, looks like yada should return a 204 if there's no body, in the case that there's no Location either (no resource has been created)

ijbriscoe10:07:18

I'll try the modified (:response ctx) approach again - is there a good example of that anywhere, as the example in the docs is only partial? I know I've done this once before successfully, but recent attempts have not been successful

ijbriscoe11:07:31

Is it essentially a Ring response? If so, wouldn't that mean I'd need to manually set the Location header?

malcolmsparks11:07:32

Yes, you assoc :status :headers and :body just like you would a Ring response. Not obvious, docs are coming. I spend about 30 mins a day now on them

ijbriscoe12:07:52

OK - I've just managed to do that, in order to fix a bug in my code for implement 404 Not Found for unknown routes.

(def catch-all
  (yada/resource {:produces "text/plain"
                  :methods  {:* {:response (fn [ctx]
                                             (assoc (:response ctx) :status 404))}}}))
where catch-all is used in something like this:
["" [["/api" ...]
     [true catch-all]]]

ijbriscoe12:07:51

Works - incidentally, is there a better way of handling 404 for all methods? Or is this pretty much as good as it gets for now?

malcolmsparks12:07:02

(as-resource nil)

ijbriscoe13:07:08

I like that - thanks.