Should we make SslOptions be more explicit about where it loads certifications?
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 38/100
Research direction
Start by reading SslOptions.java around line 245 and trace how certificate paths are currently resolved. Compare the explicit classpath and file URI cases in the issue with the existing fallback behavior, then confirm the intended compatibility rules and add coverage for the agreed resolution paths.
Written by the indexing model from the issue text.
Description
I'm not sure I like the default fallback behavior here:
Instead I recommend something more like:
static InputStream getResource(
String path)
throws FileNotFoundException, IOException {
URI uri = URI.create(path);
/*
* Explicit
*/
if ("classpath".equals(uri.getScheme())) {
var classpath = uri.getPath();
if (classpath == null) {
throw new FileNotFoundException(path);
}
return getClasspathResource(classpath);
}
if ("file".equals(uri.getScheme())) {
return Files.newInputStream(Path.of(uri));
}
/*
* Implicit
*/
Path filepath = Paths.get(path);
if (Files.exists(filepath)) {
// absolute file:
return Files.newInputStream(filepath);
}
// Maybe do not do this
return getClasspathResource(path);
}
This is where classpath:/// and file:/// can be explicitly used and then if that is not used we do the original behavior with the eventual goal of not doing the classpath unless it has the classpath uri schema.
The reason is assume I package a certification in the classpath. It works normally. Then someone adds a classpath on the filesystem when I go deploy. It now overrides the classpath one.
Now I admit this is unlikely and if this was not certs I could care less but I think we should not go around sniffing for certs. It also seems more inline with how we no longer use the service loader. That is less implicit behavior.
Otherwise I mostly don't care but just think this is the right thing to do.
- Dominant language
- Java
- Stars
- 1.8k
- Forks
- 207
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 20
Getting set up
- Ships a Dockerfile or Docker Compose file
- No pull request template
- No contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from jooby-project/jooby
-
Difficulty 5/5 Over a week Newbie friendliness 25/100
jooby-project/jooby#4021 · 5 comments ·
Maintainers usually reply within 1 day
-
Difficulty 4/5 3-5 days Newbie friendliness 25/100
jooby-project/jooby#3521 · 2 comments ·
Maintainers usually reply within 1 day
-
jooby-1.x are brokenOpenquestion
Difficulty 4/5 3-5 days Newbie friendliness 35/100
jooby-project/jooby#1513 · 10 comments ·
Maintainers usually reply within 1 day
All issues in jooby-project/jooby
Similar issues
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
breaking change bug needs human review not ready priority medium size S
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
martin-francois/symphony-trello#1193 · 1 comment ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
Maintainers usually reply within 1 day
-
Mend: dependency security vulnerability
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 70/100