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

Quarkus 2.5.0.CR1 / Final: Assets loaded from jar/classpath are not served (parsed?) correctly anymore #21602

Closed
DGuhr opened this issue Nov 22, 2021 · 22 comments · Fixed by #21655
Labels
kind/bug Something isn't working
Milestone

Comments

@DGuhr
Copy link

DGuhr commented Nov 22, 2021

Describe the bug

Hey there,

I just tried to update our Keycloak.X distribution (mutable-jar) from 2.4.2.Final to 2.5.0.Final.

Sadly, when opening the main login page with a freshly started distribution, it seems that the assets we load from classpath/jar are not loaded correctly anymore, and thus the page looks like this:
Screenshot 2021-11-22 at 08 34 54

I then checked also with 2.5.0.CR1 -> Bug also appears.

Then with same codebase from our side and 2.4.2.Final -> Assets get served correctly.

I then checked request logs -> assets get "served" with a 200 success message.

Then I checked the actual contents which are served => all served assets (images, css, js) seem to be served, but the actual content seems to be "cut-off".
So the Issue seems to be the parsing of the css/js/png files from the jar. (see screenshot in actual behaviour).

I'll try to narrow it down more now, but wanted to reach out to you people to ask if you might have an idea what could cause this new behaviour?

Expected behavior

Assets get served correctly as with 2.4.2.Final

Actual behavior

Assets (images, css, js) do not get served correctly. Looks like something has changed in the parsing of these files, resulting in them being kind of cut-off. See e.g.:
Screenshot 2021-11-22 at 08 28 59

How to Reproduce?

  1. Build Keycloak.X Distribution with pom.xml in quarkus package set to 2.5.0.Final (and postgres to 42.3.1), use e.g. mvn clean install -DskipTests -Pdistribution in root directory (or see readme here: https://github.com/keycloak/keycloak/tree/main/quarkus ).
  2. unpack zip from distribution/server-x-dist/target
  3. cd /bin
  4. run ./kc.sh start-dev
  5. open localhost:8080

(feel free to reach out to get a dist package with 2.4.2.Final/2.5.0.CR1/2.5.0.Final)

Output of uname -a or ver

Darwin dguhr-mac 20.6.0 Darwin Kernel Version 20.6.0: Tue Oct 12 18:33:42 PDT 2021; root:xnu-7195.141.8~1/RELEASE_X86_64 x86_64

Output of java -version

openjdk version "11.0.12" 2021-07-20 OpenJDK Runtime Environment Homebrew (build 11.0.12+0) OpenJDK 64-Bit Server VM Homebrew (build 11.0.12+0, mixed mode)

GraalVM version (if different from Java)

Quarkus version or git rev

2.5.0.CR1 / 2.5.0.Final

Build tool (ie. output of mvnw --version or gradlew --version)

3.6.3

Additional information

No response

@DGuhr DGuhr added the kind/bug Something isn't working label Nov 22, 2021
@geoand
Copy link
Contributor

geoand commented Nov 22, 2021

Hi,

Is there any chance you could reproduce this in a smaller example and add that to the issue?

@DGuhr
Copy link
Author

DGuhr commented Nov 22, 2021

@geoand I am still looking into the issue myself to make absolutely sure nothing's wrong "on our side".

Current assumption is it's not, bc. same code (latest main) works with quarkus 2.4.2.Final bom, and themes which we load from jar contian the right files with right contents in the dist (just checked to make sure there's nothing wrong with the jar creation approach).

Sadly it's not very easy to provide a small reproducer for this, but will talk this through with a colleague later on. Based on these assumptions, I still wanted to open the bug in hopes of someone has an "educated guess" what could cause the new behaviour for us, and what change between 2.4.2 and 2.5.0 might have caused it, so we could react accordingly.

@DGuhr DGuhr changed the title Quarkus 2.5.0.CR1 / Final: Asserts loaded from jar/classpath are not served (parsed?) correctly anymore Quarkus 2.5.0.CR1 / Final: Assets loaded from jar/classpath are not served (parsed?) correctly anymore Nov 22, 2021
@geoand
Copy link
Contributor

geoand commented Nov 22, 2021

I really think we'll need a simple reproducer for this as took a super quick look but have no idea where to find anything in the code.

@DGuhr
Copy link
Author

DGuhr commented Nov 23, 2021

Soo @geoand
Here is a working reproducer: https://github.com/DGuhr/reproducer
See steps to reproduce :) There's also the hello/fixed/... endpoint, so I think that the problem is actually our direct usage of getResourceAsStream in the response, together with a change in the internals of serving them (until 2.4.2 it worked to use getResourceAsStream directly, as we do e.g. in https://github.com/keycloak/keycloak/blob/af3b573d196af882dfb25cdccb98361746e85481/quarkus/runtime/src/main/java/org/keycloak/services/resources/QuarkusWelcomeResource.java#L157-L161, as I mentioned before).

1 similar comment
@DGuhr

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Nov 23, 2021

Thanks a lot for looking into it!

I'll have a look soon and let you know

@pedroigor
Copy link
Contributor

@geoand Do you think it could be related to any new reactive/async behavior in resteasy? To make it work @DGuhr is kind of forced to read all bytes and our requests should always run in a worker thread.

@geoand
Copy link
Contributor

geoand commented Nov 23, 2021

I have no idea TBH - I'll keep you posted.

@geoand
Copy link
Contributor

geoand commented Nov 23, 2021

So this definitely does not have anything to do with the reactive / async behavior as this issue is for RESTEasy Classic.

@DGuhr
Copy link
Author

DGuhr commented Nov 23, 2021

Considering that even

public Response test() {
        var is = new StringBufferInputStream("ciaociao");
        return Response.ok(is).build();
    }

fails, we can at least remove the classloader processing as a potential source for this behaviour.

shoutout to @andreaTP who checked this ;)

@geoand
Copy link
Contributor

geoand commented Nov 23, 2021

I have identified the Quarkus commit that causes this problem but I still need to figure out why it's problematic in this case.

@andreaTP
Copy link
Contributor

@geoand can you share the commit?

@geoand
Copy link
Contributor

geoand commented Nov 23, 2021

#20907

@geoand
Copy link
Contributor

geoand commented Nov 23, 2021

I see why it worked before and does not now, so now all that remains is to come up with a fix :)

@andreaTP
Copy link
Contributor

@geoand great! Let us know if we can support any further 🙂

@geoand
Copy link
Contributor

geoand commented Nov 23, 2021

You folks have been very helpful in shinning light on the issue :).

I'll let you know when I have a fix.

@geoand
Copy link
Contributor

geoand commented Nov 23, 2021

#21655 takes care of it

@pedroigor
Copy link
Contributor

pedroigor commented Nov 23, 2021

@geoand Awesome! Do you know about the plans for 2.5.1.Final? It would be nice to get this one and some other changes in 2.5.x before we release KC 16.

@gsmet
Copy link
Member

gsmet commented Nov 23, 2021

@pedroigor when do you need it?

We announce 2.5.0.Final tomorrow. We usually release .1.Final one week after but let me know if it works for you and if not, we can probably come up with something that works for you.

@pedroigor
Copy link
Contributor

Thanks, @gsmet . @stianst What do you think?

famod added a commit that referenced this issue Nov 24, 2021
Fix InputStream handling as Response entity in RESTEasy Classic
@quarkus-bot quarkus-bot bot added this to the 2.6 - main milestone Nov 24, 2021
@stianst
Copy link
Contributor

stianst commented Nov 24, 2021

@gsmet It's not all that urgent for us, so you don't need to go out of your way other than including this in 2.5.1 (which looks like you already are going to).

@gsmet
Copy link
Member

gsmet commented Nov 24, 2021

@stianst OK thanks. I confirm it's going to be included in 2.5.1.Final.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants