Skip to content

Conversation

dstepanov
Copy link

No description provided.

@dstepanov dstepanov requested a review from yawkat January 28, 2025 08:41
@yawkat
Copy link
Member

yawkat commented Jan 28, 2025

I don't think the client tests will work unfortunately because the server is fully embedded

@yawkat
Copy link
Member

yawkat commented Jan 28, 2025

@dstepanov you can test the fuzz target locally like this:

    public static void main(String[] args) {
        LocalJazzerRunner.create(EmbeddedHttpTarget.class).reproduce("""
            POST /redirect/echo-publisher HTTP/1.1
            Content-Length: 3

            foo
            """.stripIndent().getBytes(StandardCharsets.UTF_8));
    }

@dstepanov
Copy link
Author

it's not exposed on a port?

@yawkat
Copy link
Member

yawkat commented Jan 28, 2025

No, it's fully embedded, no tcp. It's for performance and reproducibility.

Maybe it's possible to build a tcp-based target too, but I've not explored it yet

@dstepanov
Copy link
Author

Can we start one extra server for this use case? Or this tests don't make sense? It would be nice to test the client as well

@yawkat
Copy link
Member

yawkat commented Jan 28, 2025

In principle it's possible. There's a few options:

  • Create a new target 'NioHttpTarget' which starts an actual tcp server for the test. Probably worthwhile anyway because an embedded env does not match a tcp env perfectly. But it needs some work to make it fast & reliable
  • Create a new target that specifically tests the client against a "mock" TCP server fed by fuzzer input. This could either be tcp or embedded. Would also need some thought on which client errors are "allowed" and should not trip the fuzzer.
  • In the existing EmbeddedHttpTarget, hook up the client to the existing server using an embedded channel. In core ConnectionManagerSpec kind of does this already, so it's possible.

Definitely possible but unfortunately also needs target changes.

@yawkat yawkat marked this pull request as draft January 31, 2025 10:53
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

Successfully merging this pull request may close these issues.

2 participants