Skip to content

Commit 2c5754b

Browse files
committed
fix: tomcat falls back to manifest default when Java version undetectable
When JAVA_HOME is set but the release file cannot be read (e.g. IBM JRE, non-standard layout, missing file), the previous code silently assumed Java 17 and selected Tomcat 10.x. This could break Java EE 8 apps running on Java 8 that expect Tomcat 9.x. Extract SelectTomcatVersionPattern() and return ("", nil) on detection failure so the caller falls back to DefaultVersion("tomcat") — matching the Ruby buildpack behaviour, which always deferred to the manifest default. An explicitly configured tomcat version is still honoured even without Java version info.
1 parent 40e4490 commit 2c5754b

3 files changed

Lines changed: 100 additions & 36 deletions

File tree

src/java/containers/tomcat.go

Lines changed: 42 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -63,47 +63,23 @@ func (t *TomcatContainer) Supply() error {
6363
}
6464

6565
if javaHome != "" {
66-
javaMajorVersion, versionErr := common.DetermineJavaVersion(javaHome)
66+
versionPattern, versionErr := SelectTomcatVersionPattern(javaHome, DetermineTomcatVersion(t.config.Tomcat.Version))
6767
if versionErr != nil {
68-
t.context.Log.Warning("Unable to determine Java version: %s (defaulting to 17)", versionErr.Error())
69-
javaMajorVersion = 17
68+
return versionErr
7069
}
7170

72-
tomcatVersion := DetermineTomcatVersion(t.config.Tomcat.Version)
73-
t.context.Log.Debug("Detected Java major version: %d", javaMajorVersion)
74-
75-
// Select Tomcat version pattern based on Java version
76-
var versionPattern string
77-
if tomcatVersion == "" {
78-
t.context.Log.Info("Tomcat version not specified")
79-
if javaMajorVersion >= 11 {
80-
// Java 11+: Use Tomcat 10.x (Jakarta EE 9+)
81-
versionPattern = "10.x"
82-
t.context.Log.Info("Using Tomcat 10.x for Java %d", javaMajorVersion)
83-
} else {
84-
// Java 8-10: Use Tomcat 9.x (Java EE 8)
85-
versionPattern = "9.x"
86-
t.context.Log.Info("Using Tomcat 9.x for Java %d", javaMajorVersion)
71+
if versionPattern != "" {
72+
allVersions := t.context.Manifest.AllDependencyVersions("tomcat")
73+
resolvedVersion, err := libbuildpack.FindMatchingVersion(versionPattern, allVersions)
74+
if err != nil {
75+
return fmt.Errorf("tomcat version resolution error for pattern %q: %w", versionPattern, err)
8776
}
77+
dep.Name = "tomcat"
78+
dep.Version = resolvedVersion
79+
t.context.Log.Debug("Resolved Tomcat version pattern '%s' to %s", versionPattern, resolvedVersion)
8880
} else {
89-
versionPattern = tomcatVersion
90-
t.context.Log.Info("Using Tomcat %s for Java %d", versionPattern, javaMajorVersion)
91-
}
92-
93-
if strings.HasPrefix(versionPattern, "10.") && javaMajorVersion < 11 {
94-
return fmt.Errorf("Tomcat 10.x requires Java 11+, but Java %d detected", javaMajorVersion)
95-
}
96-
97-
// Resolve the version pattern to actual version using libbuildpack
98-
allVersions := t.context.Manifest.AllDependencyVersions("tomcat")
99-
resolvedVersion, err := libbuildpack.FindMatchingVersion(versionPattern, allVersions)
100-
if err != nil {
101-
return fmt.Errorf("tomcat version resolution error for pattern %q: %w", versionPattern, err)
81+
t.context.Log.Warning("Unable to determine Java version from JAVA_HOME, falling back to manifest default Tomcat version")
10282
}
103-
104-
dep.Name = "tomcat"
105-
dep.Version = resolvedVersion
106-
t.context.Log.Debug("Resolved Tomcat version pattern '%s' to %s", versionPattern, resolvedVersion)
10783
}
10884

10985
// Fallback to default version if we couldn't determine Java version
@@ -463,7 +439,37 @@ func getKeys(m map[string]string) []string {
463439
return keys
464440
}
465441

466-
// DetermineTomcatVersion determines the version of the tomcat
442+
// SelectTomcatVersionPattern determines the Tomcat version pattern to use based on the
443+
// detected Java version and any user-configured Tomcat version.
444+
// Returns ("", nil) when Java version cannot be determined — the caller should fall back
445+
// to the manifest default (matching Ruby buildpack behaviour).
446+
func SelectTomcatVersionPattern(javaHome, configVersion string) (string, error) {
447+
if javaHome == "" {
448+
return "", nil
449+
}
450+
451+
javaMajorVersion, err := common.DetermineJavaVersion(javaHome)
452+
if err != nil {
453+
if configVersion != "" {
454+
return configVersion, nil
455+
}
456+
return "", nil
457+
}
458+
459+
if configVersion != "" {
460+
if strings.HasPrefix(configVersion, "10.") && javaMajorVersion < 11 {
461+
return "", fmt.Errorf("Tomcat 10.x requires Java 11+, but Java %d detected", javaMajorVersion)
462+
}
463+
return configVersion, nil
464+
}
465+
466+
if javaMajorVersion >= 11 {
467+
return "10.x", nil
468+
}
469+
return "9.x", nil
470+
}
471+
472+
467473
// based on the JBP_CONFIG_TOMCAT field from manifest.
468474
// It looks for a tomcat block with a version of the form "<major>.+" (e.g. "9.+", "10.+", "10.1.+").
469475
// Returns the pattern with "+" replaced by "*" (e.g. "9.*", "10.*", "10.1.*") so libbuildpack can resolve it.

src/java/containers/tomcat_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,62 @@ var _ = Describe("Tomcat Container", func() {
197197
})
198198
})
199199

200+
Describe("SelectTomcatVersionPattern", func() {
201+
var javaHome string
202+
203+
BeforeEach(func() {
204+
var err error
205+
javaHome, err = os.MkdirTemp("", "javahome")
206+
Expect(err).NotTo(HaveOccurred())
207+
})
208+
209+
AfterEach(func() {
210+
os.RemoveAll(javaHome)
211+
})
212+
213+
writeReleaseFile := func(content string) {
214+
err := os.WriteFile(filepath.Join(javaHome, "release"), []byte(content), 0644)
215+
Expect(err).NotTo(HaveOccurred())
216+
}
217+
218+
Context("when release file is missing", func() {
219+
It("returns empty pattern to fall back to manifest default, not assume Java 17", func() {
220+
pattern, err := containers.SelectTomcatVersionPattern(javaHome, "")
221+
Expect(err).NotTo(HaveOccurred())
222+
Expect(pattern).To(Equal(""))
223+
})
224+
225+
It("still honours an explicitly configured tomcat version", func() {
226+
pattern, err := containers.SelectTomcatVersionPattern(javaHome, "9.*")
227+
Expect(err).NotTo(HaveOccurred())
228+
Expect(pattern).To(Equal("9.*"))
229+
})
230+
})
231+
232+
Context("happy path version selection", func() {
233+
It("selects Tomcat 10.x for Java 11+", func() {
234+
writeReleaseFile("JAVA_VERSION=\"11.0.20\"\n")
235+
pattern, err := containers.SelectTomcatVersionPattern(javaHome, "")
236+
Expect(err).NotTo(HaveOccurred())
237+
Expect(pattern).To(Equal("10.x"))
238+
})
239+
240+
It("selects Tomcat 9.x for Java 8", func() {
241+
writeReleaseFile("JAVA_VERSION=\"1.8.0_372\"\n")
242+
pattern, err := containers.SelectTomcatVersionPattern(javaHome, "")
243+
Expect(err).NotTo(HaveOccurred())
244+
Expect(pattern).To(Equal("9.x"))
245+
})
246+
247+
It("errors when Tomcat 10.x is requested but Java 8 detected", func() {
248+
writeReleaseFile("JAVA_VERSION=\"1.8.0_372\"\n")
249+
_, err := containers.SelectTomcatVersionPattern(javaHome, "10.*")
250+
Expect(err).To(HaveOccurred())
251+
Expect(err.Error()).To(ContainSubstring("Java 11+"))
252+
})
253+
})
254+
})
255+
200256
Describe("determineTomcatVersion", func() {
201257
It("returns empty string when JBP_CONFIG_TOMCAT is empty", func() {
202258
v := containers.DetermineTomcatVersion("")

src/java/supply/supply_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,8 @@ dependencies: []
107107
Expect(os.MkdirAll(filepath.Join(jdkInstallDir, "jre-17.0.15", "bin"), 0755)).To(Succeed())
108108
javaBin := filepath.Join(jdkInstallDir, "jre-17.0.15", "bin", "java")
109109
Expect(os.WriteFile(javaBin, []byte("mockfile"), 0644)).To(Succeed())
110+
releaseFile := filepath.Join(jdkInstallDir, "jre-17.0.15", "release")
111+
Expect(os.WriteFile(releaseFile, []byte("JAVA_VERSION=\"17.0.15\"\n"), 0644)).To(Succeed())
110112

111113
// adjust JRE component mocks used during supply
112114
depJre := libbuildpack.Dependency{Name: "openjdk", Version: "17.0.15"}

0 commit comments

Comments
 (0)