@RequestParam default value not set in certain cases [SPR-10180]

[spring-projects-issues]

Michael Osipov opened SPR-10180 and commented

I have this @RequestMapping

@RequestMapping(value = "/{project:[A-Z0-9_+\\.\\(\\)=\\- ]+}", method = RequestMethod.GET, produces = {
    MediaType.APPLICATION_XML_VALUE, "application/json;charset=UTF-8" })
public ResponseEntity<Object> lookupProject(@PathVariable String project,
    @RequestParam(value = "attributes", required = false) String[] attributes,
    @RequestParam(value = "outputType", required = false, defaultValue = "HASH") OutputType outputType)

Where OutputType is an enum with the values HASH and ARRAY.

In the following edge cases:

GET /.../TEST-PROJECT?outputType HTTP/1.1
GET /.../TEST-PROJECT?outputType= HTTP/1.1

The outputType variable is not populated with the default value but a null is assigned. The RequestParamMethodArgumentResolver receives this as an empty string.

See the attached screenshot of the debugger.

If this cannot be reasonable fixed, add at least a paragraph to the docs that such edge cases exist.

---

Affects: 3.1.3, 3.2 GA

Attachments:

• empty value.png (119.78 kB)

Referenced from: commits 3abe05c, 221562d

[spring-projects-issues]

Rossen Stoyanchev commented

We can use the default value in those cases. Injecting null when a default value is specified doesn't make much sense.

[spring-projects-issues]

Michael Osipov commented

Rossen, that makes sense but the general question is how should we treat such parameters (values) at all? What are these states?

project?outputType => none given?
project?outputType => intentional empty value?

[spring-projects-issues]

Rossen Stoyanchev commented

If a request parameter is present but is empty (i.e. "project?outputType" or "project?outputType="), type conversion turns the empty string into null. It doesn't seem to make much sense injecting null when the @RequestParam annotation has a default value. Or do you actually expect to see null in that case?

[spring-projects-issues]

Michael Osipov commented

What I would expect regardless the API does support that or not:

Default value given and not required:
project?outputType => Exception (missing value)
project?outputType= => Exception (missing value)

No default value given and not required:
remains null in either case => fine

No default value given and required:
project?outputType => Exception (missing value)
project?outputType= => Exception (missing value)

[spring-projects-issues]

Rossen Stoyanchev commented

Default value given and not required:
project?outputType => Exception (missing value)
project?outputType= => Exception (missing value)

That would be useful if you want to validate that the client always provides a non-empty value or no value at all. Another application however might not care whether the value is empty or missing, and would expect the default value to be used always.

Since there is no way to meet both of requirements, it would be good to make sure they can be achieved in some way. This is why I don't favor always raising an exception, which is in essence what you proposed. Instead if we tightened up the default value to be used for both empty and missing values, we'd get a single intuitive behavior for the default value, and if you wanted to treat empty values as an error, then you could remove the default value from @RequestParam and then check/assign it inside the controller method. Not as pretty but at least there is an option. Does that make sense?

[spring-projects-issues]

Rossen Stoyanchev commented

Moving to 3.2.2 to allow more time for comments.

[spring-projects-issues]

Rossen Stoyanchev commented

Changing to 'Improvement'. The current behavior is a special case for which the behavior is not well defined rather than a bug.

[spring-projects-issues]

Michael Osipov commented

Rossen, yes your argumentation makes sense. I would accept your proposed behavior as a good choice. How would you comment on the last case? Would agree?

I any way, we would have a clear/uniform behavior how edge cases would be treated.

[spring-projects-issues]

Michael Osipov commented

Thanks for the fix. Are you able to reflect this change in the @RequestParam JavaDoc?

[spring-projects-issues]

Rossen Stoyanchev commented

Yes, thanks for the reminder.