nicferrier / elnode

evented io webserver right inside your emacs.
http://nicferrier.github.com/elnode
GNU General Public License v3.0
477 stars 49 forks source link

changed regex in elnode--http-parse-status to support arbitrary methods #66

Closed sabof closed 11 years ago

sabof commented 11 years ago

I'm not sure how servers are supposed to handle non-standard methods. I'd think passing them to the application would be better, at least from the forward-compatibility point of view. An alternative would be to extend the regex to handle all standard methods.

Btw, I had an issue with elnode, where emacs would send a blank response, and not use the handler. Maybe once every 70 requests. I'm not even sure whether it gets to the filter stage. Is this a known problem?

nicferrier commented 11 years ago

No, dropped requests are not a known problem. Are you sure they're getting to elnode?

As to the pull request: you're absolutely right it should at least support DELETE and OPTIONS and so on... I am no at all sure about supporting arbitary methods. That would encourage all sorts of breakage surely?

Maybe we could set something on the request to allow unsupported methods?

What's your use case?

sabof commented 11 years ago

No, I'm not. I suppose I could dump filter input somewhere, to see what happens. They are ajax requests sent from google chrome.

I don't know. It's not a hot issue right now, but it might become one, once computing gets past the "Let's making everything RESTful" stage. If it ever does. I'm trying to implement a backend for a backbone.js tutorial - it's not something that affects me.

nicferrier commented 11 years ago

notifications@github.com writes:

No, I'm not. I suppose I could dump filter input somewhere, to see what happens. They are ajax requests sent from google chrome.

Or check the log buffers?

nicferrier commented 11 years ago

@sabof I can't accept this as it is, but I would accept a change to elnode-start that would turn this on for a particular server:

(elnode-start handler :port 8091 :allow-any-method t)

or possibly even:

(elnode-start handler :port 8091 :allow-method-regex "\\(GET\|POST\\)")

that would be a good interface to do what you want?

sabof commented 11 years ago

I just liked the "pass through" idea. I don't think there is enough demand to make it into an option. In any case it's easy to redefine the function, should a need arise.

nicferrier commented 11 years ago

I think it would be good to add, personally. But I don't think it should be the default. I'd like it, but it's not my top priority.

If you don't do it I'll close this PR and make it a low priority issue.

sabof commented 11 years ago

I think supporting all the standard methods is important. I can update the regex with those if you wish. I suppose the "allowed-method-regex" also has some appeal. I'll have a look at it.

nicferrier commented 11 years ago

Yeah. Ok. Again, I'll do it if you don't.

sabof commented 11 years ago

I've added put and delete to the default regex, as these are fairly common, and should be handled by the application.

nicferrier commented 11 years ago

thanks for that.