Skip to content

Add support for using colon as a separator for custom methods #24771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
deepak-auto opened this issue Mar 24, 2020 · 4 comments
Closed

Add support for using colon as a separator for custom methods #24771

deepak-auto opened this issue Mar 24, 2020 · 4 comments
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@deepak-auto
Copy link

deepak-auto commented Mar 24, 2020

Affects: 5.1.7

Google's API Design Guide has the concept of a custom method which is identified by using the colon character :. The colon character is used to separate the custom verb from the resource.

Spring-WebFlux currently treats colon as any other character. Although it is possible to create custom method endpoints using the version mentioned above, we run into weird errors such as:

  1. If we have two endpoints GET /resources/{id} and POST /resources/{id}:customVerb, and the user mistakenly sends a request to GET /resources/{id}:customVerb, the GET /resources/{id} endpoint gets hit instead of returning a 405 error
  2. If we put a @RequestMapping(path = "/resources") on the controller and a PostMapping(path = ":customVerb" on the method, it gets mapped to POST /resources/:customVerb instead of POST /resources:customVerb. A workaround we have found for this was to stop using the @RequestMapping annotation and instead put the full path in the custom method's mapping itself.

So, it would be great if the the team at Spring could provide first class support to the colon character as a custom method separator.

Thanks!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 24, 2020
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Mar 25, 2020

The choice to append a custom HTTP method to the path does not seem like a good idea. It is very reminiscent of expressing acceptable content types via path extensions and see #24179 how that turned out.

I see a lot of the same issues play out here as well. For example in a URL such as "/something/{id}" the id could contain ":" for reasons unrelated to a custom HTTP method leading to ambiguity. Then you have to configure known custom methods in order to disambiguate. Similar treatment would be required in a security framework that looks at URLs for authorization purposes, and any mismatch between security and framework mapping leads to security issues. Not to mention possibilities for a malicious user to exploit such URLs. And how about trying to overlay the handling of such custom HTTP methods with the support for path extensions and path parameters. Let's not bring in URL encoding issues too which adds a whole new layer.

There is enough complexity in dealing with paths. I would strongly discourage overloading it with yet another concept. Have you considered passing the custom method as a query parameter?

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Mar 25, 2020
@deepak-auto
Copy link
Author

Thank you for your reply.

I understand your concerns regarding supporting a specific requirement coming out of the specifications from a certain company into "general purpose" Spring Web frameworks. As one of my colleagues observed, maybe Google has more control over their APIs which allows them to have the colon character to mean something special. So, it only makes sense that we handle the weird edge cases resulting from the Google standard on our end as opposed to expecting everyone who uses Spring Web frameworks to treat colons a separator for custom methods.

@rstoyanchev rstoyanchev added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 26, 2020
@rstoyanchev
Copy link
Contributor

No worries, I'm not familiar enough with the Google API but indeed they can make many more assumptions about the URLs. Still I don't think it's a good idea to do it this way, and with sufficient numbers of users and use cases eventually the issues surface.

@rstoyanchev rstoyanchev self-assigned this Mar 26, 2020
@ahmeddossamaa
Copy link

Hey @deepak-auto , saw your comments and was curios, what's the alternative approach that you took to achieve this scalability in design? I wanted to use colons badly but Spring didn't help me either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

4 participants