Update schema to accept string HTTP methods like "GET", as well as keywords.

[danielcompton]

The changes in release 0.6.2 broke some production code which was using :request-method "GET" instead of :request-method :get. I thought it might be good to warn others about this.

[KingMob]

@danielcompton Thanks for pointing this out. At first, I was surprised that string methods even worked, but I took a look at the code, and the transformation from keyword to Netty input is such that strings will usually work, too. (Though string TRACE methods will break some code.) FWIW, the Ring SPEC is clear, and it's been keywords-only for its entire life.

Regardless, I guess the question is, should we:

1. Add a breaking change note, like this PR?
2. Remove wrap-validation from the default middleware, and silently continue to accept strings?
3. Loosen the schema to accept strings, too?

I'd rather not break any other wild code, so I prefer 2 or 3. I think 2 will leave the validation effectively unused by 99.99% of users, which defeats its purpose. I still hope to tighten the schema for areas like the query-string and the uri, so I vote for 3. This is not a strongly-held opinion, though. Thoughts?

---

From #679:

We should probably tighten the schemas soon. I've had bad experiences in the past using spec, when people let specs be loose for too long, they became concerned that locking them down would break something (E.g., one team only accepts ports as numbers, but another team also accepts them as strings which it parses...)

Isn't it too late already?

Well, that comment exchange proved prophetic 😂

[danielcompton]

I'd rather not break any other wild code, so I prefer 2 or 3. I think 2 will leave the validation effectively unused by 99.99% of users, which defeats its purpose. I still hope to tighten the schema for areas like the query-string and the uri, so I vote for 3. This is not a strongly-held opinion, though. Thoughts?

Yeah, 3 seems like a safe middle ground without breaking existing code when people upgrade Aleph versions.

A possible extension to 3 could be to log a warning when callers pass a string instead of a keyword. That way, you could still tighten the spec one day if you ever decided to. Although I don't really know what you'd be gaining at that point, so maybe allowing string + keyword is ok.

[arnaudgeiser]

Loosen the schema to accept strings, too?

I would definitely loosen the schema. And as a follow-up, confirm we didn't break any other parameters along the way.

[KingMob]

Option 3 it is.

Daniel, do you want to tackle it, since you have the PR open? It's fine if not, I'm just saying you're welcome to if you like.

[danielcompton]

Sorry, I dropped this. Happy for you to take it if you'd like.

[KingMob]

@danielcompton We can do it, but I have plenty on my plate at the moment, and you're welcome to if you like. Just let us know.

[danielcompton]

@KingMob I've updated the PR to expand the validation. LMK what you think.

[arnaudgeiser]

It looks good to me, thank you Daniel!

[KingMob]

Just got back from vacation. Looks cool to me. (Tho testing needs to be required)

My only note is that the Ring spec never accepted strings, and the changelog shouldn't say otherwise. It's just an Aleph bug that users might have come to rely on.