Skip to content
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

Non-nullable Long Kotlin data class field initialized with 0 when field is missing in payload #880

Closed
erkieh opened this issue Jun 8, 2024 · 3 comments

Comments

@erkieh
Copy link

erkieh commented Jun 8, 2024

Expected Behavior

Posting validated HTTP JSON Payloads with missing values for Non-nullable Long Kotlin data class fields should result in a 400 bad request response,

Actual Behaviour

Non-nullable Long Kotlin data class fields are initialized with 0 when the field is missing in the payload.
Strings for example behave differently. They don't get defaulted. Instead the result will be an 500 Internal server error with a NullPointerException happening on the server side.

Steps To Reproduce

Run tests in sample app

Environment Information

Windows 10 Java 21

Example Application

git@github.com:erkieh/demonullhandling.git

Version

4.5.0-SNAPSHOT

@roar-skinderviken
Copy link

roar-skinderviken commented Jun 25, 2024

This issue seems to be related to this issue except for that

  • missing String-value in JSON causes NPE, while missing Long-value like in this issue, gives Long default value (0)
  • this behavior seems to be unchanged across MN:4.x versions

Please note that any default values in Kotlin data classes are ignored during deserialization in this issue.

A possible workaround for returning 400 bad request when Long-value is missing in JSON, may be to use @Min along with @Bindable annotations (given that only non-negative numbers are valid):

import io.micronaut.core.bind.annotation.Bindable
import io.micronaut.serde.annotation.Serdeable
import jakarta.validation.constraints.Min

@Serdeable
data class NonNullDto(
    @Min(0L)
    @Bindable(defaultValue = "-1")
    val longField: Long
)

Tests

import com.deserializenull.rest.dto.NonNullDto
import com.deserializenull.rest.dto.NullDto
import io.kotest.assertions.throwables.shouldThrow
import io.kotest.core.spec.style.StringSpec
import io.kotest.matchers.shouldBe
import io.micronaut.http.HttpRequest
import io.micronaut.http.client.HttpClient
import io.micronaut.http.client.annotation.Client
import io.micronaut.http.client.exceptions.HttpClientResponseException
import io.micronaut.test.extensions.kotest5.annotation.MicronautTest

@MicronautTest
class NonNullDtoControllerTest(
    @Client("/") httpClient: HttpClient
) : StringSpec({

    "test deserialization with null data, expect 400 response" {
        val exception = shouldThrow<HttpClientResponseException> {
            httpClient.toBlocking().exchange(HttpRequest.POST("/nonnulldto", NullDto()), NullDto::class.java)
        }

        exception.status.code shouldBe 400
    }

    "test deserialization with valid data, expect 200 response" {
        val response = httpClient.toBlocking().exchange(HttpRequest.POST("/nonnulldto", NonNullDto(longField = 1)), NonNullDto::class.java)
        val responseBody = response.getBody(NonNullDto::class.java).get()

        response.status.code shouldBe 200
        responseBody.longField shouldBe 1
    }
})

@dstepanov
Copy link
Contributor

Please create / update the sample app with your use case

@dstepanov
Copy link
Contributor

The behaviour is the same as a default in Jackson. The fix is a configuration to fail on null primitives.

graemerocher pushed a commit to micronaut-projects/micronaut-core that referenced this issue Jun 26, 2024
The test is reproducing Jackson's default behaviour with primitives and a way to change it

micronaut-projects/micronaut-serialization#880
yawkat added a commit to micronaut-projects/micronaut-core that referenced this issue Jul 17, 2024
* fix(deps): update netty monorepo to v4.1.111.final (#10905)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Ensuring the deserialization has access to the ServerRequestContext (#10904)

* Fixes #10665 Ensuring the deserialization has access to the ServerRequestContext

* Adding a test to ensure that large request payloads still have access to the ServerRequestContext on deserialization.

* Map of beans should always use bean definition to extract the name (#10908)

* Improve the ControllerConstraintHandlerTest to accept more HttpRequest body types (#10909)

* Add Kotlin+KSP+Jackson nullable tests (#10927)

The test is reproducing Jackson's default behaviour with primitives and a way to change it

micronaut-projects/micronaut-serialization#880

* Update common files (#10923)

* Revert "Improve the ControllerConstraintHandlerTest to accept more HttpReques…" (#10922)

This reverts commit 3744f17.

* chore(deps): update plugin io.micronaut.build.shared.settings to v6.7.1 (#10770)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix(deps): update dependency io.github.classgraph:classgraph to v4.8.174 (#8882)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix(deps): update vertx to v4.5.8 (#10863)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix(deps): update groovy monorepo to v4.0.22 (#10939)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update dependency me.champeau.gradle:japicmp-gradle-plugin to v0.4.3 (#10885)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix(deps): update junit5 monorepo (#10945)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update Micronaut Maven Plugin coordinates (#10946)

[skip ci]

* include all non-accessible methods in reflection data (#10947)

Fixes #10880

* Fix CompletableFuture responses without body (#10953)

Fixes #10917

* fix(deps): update dependency com.fasterxml.jackson.dataformat:jackson-dataformat-yaml to v2.17.2 (#10954)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Merge remote-tracking branch 'origin/4.5.x' into merge_4_5_6

# Conflicts:
#	http-server-netty/src/main/java/io/micronaut/http/server/netty/binders/NettyBodyAnnotationBinder.java
#	inject/src/main/java/io/micronaut/context/DefaultBeanContext.java

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Luís Serralheiro <encavadas@gmail.com>
Co-authored-by: Andriy Dmytruk <80816836+andriy-dmytruk@users.noreply.github.com>
Co-authored-by: micronaut-build <65172877+micronaut-build@users.noreply.github.com>
Co-authored-by: Jonas Konrad <jonas.konrad@oracle.com>
Co-authored-by: Álvaro Sánchez-Mariscal <alvaro.sanchez-mariscal@oracle.com>
Co-authored-by: Graeme Rocher <graeme.rocher@oracle.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants