jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.8k stars 1.9k forks source link

External property file not being read #12019

Closed Joellao closed 2 weeks ago

Joellao commented 2 weeks ago

Jetty Version 12.0.7

Jetty Environment: EE9

Java 17

Hello everyone, I updated from Jetty 10 to Jetty 12. With some tweaks around I was able to have everything working. I was using an external properties file in order to change the ssl port without changing anything related to jetty ".ini" files.

Then the whole application was being started through java -jar start.jar resources/jetty-settings.properties

Jetty-settings.properties is in the resources folder with the jetty-logging.properties

That seems to not be working anymore. I know that I can pass system parameter to the command with jetty.ssl.port being set and that will work and I also know that setting those through the .ini files is working just fine.

Is there something that I'm missing regarding this? Maybe something changed and I didn't find it online?

When I do --list-config I get ${jetty.base}/resources/jetty-settings.properties:jetty.ssl.port = 8312 so somehow it's being recognized, but not being used.

I also found this https://github.com/jetty/jetty.project/issues/2719 so this approach "should" work fine

Thanks in advance

joakime commented 2 weeks ago

I was using an external properties file in order to change the ssl port without changing anything related to jetty ".ini" files.

A properties file is not necessary here. Just give the property to the command line.

Eg:

java -jar start.jar jetty.ssl.port=8132
Joellao commented 2 weeks ago

I was using an external properties file in order to change the ssl port without changing anything related to jetty ".ini" files.

A properties file is not necessary here. Just give the property to the command line.

Eg:

java -jar start.jar jetty.ssl.port=8132

Yeah I know that I can dot that, as I said in the original post, but the use case is a little different.

I would need to change a "core" part by introducing this simple fix. In the worst case that's what is going to happen, but before that, I'd like to know if it's still possible or something changed. Or maybe it's something caused by the migration.

Cheers

janbartel commented 2 weeks ago

@Joellao although this isn't a documented feature (at least we don't seem to output anything about it in the --help output), it looks to me that it should still be working. Try turning on --debug when you run, and see if you can spot the parsing of your properties file in the output.

Joellao commented 2 weeks ago

@janbartel Yes, that's true that it isn't a documented thing, and I would also kinda understand if that was removed. When I turn the --debug one it is doing a parsing: DEBUG : parse("resources/jetty-settings.properties", "<command-line>")

The only difference is that in Jetty 10 it's adding it as a last entry in the command line, instead in the Jetty 12 it's adding it before the classpath entries (multiple one, instead of a big chunk of paths). jetty-ee9-webapp.xml and jetty-ee-deploy.xml are the last entries for Jetty 12

Joellao commented 2 weeks ago

I tried with 12.0.11 and it seems to be working on that version. Didn't do anything fancy, just starting with server and http modules enabled. Passed the property file as an argument and indeed it starts the server in the specified port. Strange thing

joakime commented 2 weeks ago

I tried with 12.0.11 and it seems to be working on that version. Didn't do anything fancy, just starting with server and http modules enabled. Passed the property file as an argument and indeed it starts the server in the specified port. Strange thing

There's been no changes to jetty-start behavior related to properties files in any Jetty 12.0.x version.

I reviewed all of the changes with ...

[jetty-12.0.x]$ git diff jetty-12.0.0...jetty-12.0.11 -- jetty-core/jetty-start/

The fact that it works for you with 12.0.11 and not with 12.0.7 is just strange.

Joellao commented 2 weeks ago

@joakime Indeed it's not working. One of the demos was using port 8888 in http.ini file instead of 8080 so I celebrated too early. Now I did a proper test on that and it's not working as expected. But in 10.0.22 it is working

janbartel commented 2 weeks ago

@Joellao there is a problem in jetty-12, I can now see that the properties file is not being passed into XmlConfiguration. Let me look into this, I'll get back to you.

janbartel commented 2 weeks ago

Issue will be fixed for 2.0.12 via #12026

Joellao commented 2 weeks ago

@janbartel Thank you very much for the insights and the prompt fix. Really glad that I'll need to only perform an upgrade to the newest version instead of changing a core part of the application. Best regards