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:dev hot reload changes default charset? #3467

Closed
seseso opened this issue Aug 9, 2019 · 7 comments
Closed

quarkus:dev hot reload changes default charset? #3467

seseso opened this issue Aug 9, 2019 · 7 comments
Assignees
Labels
area/devmode kind/bug Something isn't working
Milestone

Comments

@seseso
Copy link

seseso commented Aug 9, 2019

Running a project with quarkus:dev, everything works fine. When triggering hot reload (changing some source), the behavior changes and Strings that exists in the code seems to be interpreted in a different charset.

I created a small project to reproduce this error: https://github.com/seseso/quarkus-hot-reload
Steps:

  1. mvn clean install -DskipTests quarkus:dev
  2. curl http://localhost:8085/hello/S%C3%A9rgio
  3. Excpected output: hello Boss!
  4. Change a source and save, triggering hot reload
  5. curl http://localhost:8085/hello/S%C3%A9rgio
  6. Excpected output: hello Boss! Actually, it returns Hello Sérgio

The rest endpoint is this one:

@GET
@Path("/{name}")
@Produces(MediaType.TEXT_PLAIN)
public String hello(@PathParam("name") String name) {
    
    if("Sérgio".equals(name)) {
        return "hello Boss!";
    }
    
    return "hello " + name;
}

So, after hot reload, the if does not matches.

Environment

  • Output of uname -a or ver: Windows 10 PRO
  • Output of java -version: openjdk version "1.8.0_212"
  • GraalVM version (if different from Java): Not using
  • Quarkus version or git rev: 0.20.0

If more information is needed, please let me know as I'll be glad to provide...

@seseso seseso added the kind/bug Something isn't working label Aug 9, 2019
@seseso seseso changed the title quarkus:dev hot reload chages default charset? quarkus:dev hot reload changes default charset? Aug 9, 2019
@gwenneg
Copy link
Member

gwenneg commented Aug 9, 2019

Hi @seseso! I just tried to reproduce this using your project on a MacBook but the curl call always returned hello Boss!. Could you please verify that the source file encoding is not changed when you edit it? That could also be a Windows specific issue.

You can check the file encoding using the file * command from the source directory if you're using Git bash from Windows.

@seseso
Copy link
Author

seseso commented Aug 9, 2019

I doubt that the save action do a change on charset as if I stop / start the project, everything works fine again.
I bet more on a Windows specific issue. Tested on a WSL Linux in my Windows machine and everything works fine also.

@gwenneg
Copy link
Member

gwenneg commented Aug 9, 2019

Yes I agree the charset change on save was very unlikely but since it can happen, I had to ask :)

Thanks for testing this with Linux!

@gwenneg
Copy link
Member

gwenneg commented Aug 9, 2019

I'm not sure this is related but during a hot reload, we're recompiling the changed sources here using a StandardJavaFileManager which relies on the platform default charset because we're not providing a specific charset (which does not look like a good idea at first glance).

I'll assign this issue to myself if I'm able to reproduce it on my mac.

@gwenneg
Copy link
Member

gwenneg commented Aug 9, 2019

I forced a wrong charset and obtained similar results during a hot reload with your project, but I couldn't exactly reproduce the issue since I don't have any Windows computer to test it right now.

@geoand: WDYT of the fix in the PR? I'm not sure it's ok to have a hardcoded charset in JavaCompilationProvider since the charset usually comes from the project.build.sourceEncoding Maven property or a Gradle equivalent for the initial compilation. I could try retrieving the charset from the Maven/Gradle plugins system properties if the hardcoded one is not ok.

@geoand
Copy link
Contributor

geoand commented Aug 9, 2019

@gwenneg I think that it would be OK to have UTF8 for now since that is by far the most common scenario.

@gwenneg
Copy link
Member

gwenneg commented Aug 13, 2019

Fixed in #3471 and #3487.

@gwenneg gwenneg closed this as completed Aug 13, 2019
@gsmet gsmet added this to the 0.21.0 milestone Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devmode kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants