Mark,
On 4/29/25 3:50 AM, ma...@apache.org wrote:
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/11.0.x by this push:
new fab7247d2f Refactor CGI servlet to access resources via WebResources
fab7247d2f is described below
commit fab7247d2f0e3a29d5daef565f829f383e10e5e2
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Apr 28 12:58:21 2025 +0100
Refactor CGI servlet to access resources via WebResources
---
java/org/apache/catalina/servlets/CGIServlet.java | 233 ++++++++-------------
.../catalina/servlets/LocalStrings.properties | 1 -
.../catalina/servlets/LocalStrings_fr.properties | 1 -
.../catalina/servlets/LocalStrings_ja.properties | 1 -
.../catalina/servlets/LocalStrings_ko.properties | 1 -
.../servlets/LocalStrings_zh_CN.properties | 1 -
webapps/docs/changelog.xml | 4 +
7 files changed, 97 insertions(+), 145 deletions(-)
diff --git a/java/org/apache/catalina/servlets/CGIServlet.java
b/java/org/apache/catalina/servlets/CGIServlet.java
index 510d2782e7..db03ae1955 100644
--- a/java/org/apache/catalina/servlets/CGIServlet.java
+++ b/java/org/apache/catalina/servlets/CGIServlet.java
@@ -25,6 +25,8 @@ import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.Serial;
import java.io.UnsupportedEncodingException;
+import java.net.MalformedURLException;
+import java.net.URL;
import java.net.URLDecoder;
import java.nio.file.Files;
import java.util.ArrayList;
@@ -628,9 +630,6 @@ public final class CGIServlet extends HttpServlet {
/** pathInfo for the current request */
private String pathInfo = null;
- /** real file system directory of the enclosing servlet's web app */
- private String webAppRootDir = null;
-
/** tempdir for context - used to expand scripts in unexpanded wars */
private File tmpDir = null;
@@ -684,7 +683,6 @@ public final class CGIServlet extends HttpServlet {
*/
protected void setupFromContext(ServletContext context) {
this.context = context;
- this.webAppRootDir = context.getRealPath("/");
this.tmpDir = (File) context.getAttribute(ServletContext.TEMPDIR);
}
@@ -788,10 +786,9 @@ public final class CGIServlet extends HttpServlet {
* cgiPathPrefix is defined by setting this servlet's cgiPathPrefix
init parameter
* </p>
*
- * @param pathInfo String from HttpServletRequest.getPathInfo()
- * @param webAppRootDir String from context.getRealPath("/")
* @param contextPath String as from
HttpServletRequest.getContextPath()
* @param servletPath String as from
HttpServletRequest.getServletPath()
+ * @param pathInfo String from HttpServletRequest.getPathInfo()
* @param cgiPathPrefix subdirectory of webAppRootDir below which the
web app's CGIs may be stored; can be null.
* The CGI search path will start at
webAppRootDir + File.separator + cgiPathPrefix (or
* webAppRootDir alone if cgiPathPrefix is
null). cgiPathPrefix is defined by setting
@@ -808,59 +805,108 @@ public final class CGIServlet extends HttpServlet {
* found
* </ul>
*/
- protected String[] findCGI(String pathInfo, String webAppRootDir,
String contextPath, String servletPath,
- String cgiPathPrefix) {
- String path;
- String name;
- String scriptname;
-
- if (webAppRootDir.lastIndexOf(File.separator) ==
(webAppRootDir.length() - 1)) {
- // strip the trailing "/" from the webAppRootDir
- webAppRootDir = webAppRootDir.substring(0,
(webAppRootDir.length() - 1));
- }
+ protected String[] findCGI(String contextPath, String servletPath,
String pathInfo, String cgiPathPrefix) {
I know it wasn't your goal to clean any of this up, but I think a custom
class would be better for this application than String[].
Any objections to be re-factoring this to use a new class?
- if (cgiPathPrefix != null) {
- webAppRootDir = webAppRootDir + File.separator + cgiPathPrefix;
- }
+ StringBuilder cgiPath = new StringBuilder();
+ StringBuilder urlPath = new StringBuilder();
- if (log.isTraceEnabled()) {
- log.trace(sm.getString("cgiServlet.find.path", pathInfo,
webAppRootDir));
- }
+ URL cgiScriptURL = null;
- File currentLocation = new File(webAppRootDir);
- StringTokenizer dirWalker = new StringTokenizer(pathInfo, "/");
- if (log.isTraceEnabled()) {
- log.trace(sm.getString("cgiServlet.find.location",
currentLocation.getAbsolutePath()));
+ if (cgiPathPrefix == null || cgiPathPrefix.isEmpty()) {
+ cgiPath.append(servletPath);
+ } else {
+ cgiPath.append('/');
+ cgiPath.append(cgiPathPrefix);
}
- StringBuilder cginameBuilder = new StringBuilder();
- while (!currentLocation.isFile() && dirWalker.hasMoreElements()) {
- String nextElement = (String) dirWalker.nextElement();
- currentLocation = new File(currentLocation, nextElement);
- cginameBuilder.append('/').append(nextElement);
+ urlPath.append(servletPath);
+
+ StringTokenizer pathWalker = new StringTokenizer(pathInfo, "/");
+
+ while (pathWalker.hasMoreElements() && cgiScriptURL == null) {
+ String urlSegment = pathWalker.nextToken();
+ cgiPath.append('/');
+ cgiPath.append(urlSegment);
+ urlPath.append('/');
+ urlPath.append(urlSegment);
if (log.isTraceEnabled()) {
- log.trace(sm.getString("cgiServlet.find.location",
currentLocation.getAbsolutePath()));
+ log.trace(sm.getString("cgiServlet.find.location",
cgiPath.toString()));
+ }
+ try {
+ cgiScriptURL = context.getResource(cgiPath.toString());
+ } catch (MalformedURLException e) {
+ // Ignore - should never happen
}
}
- String cginame = cginameBuilder.toString();
- if (!currentLocation.isFile()) {
+
+ // No script was found
+ if (cgiScriptURL == null) {
return new String[] { null, null, null, null };
}
- path = currentLocation.getAbsolutePath();
- name = currentLocation.getName();
+ // Set-up return values
+ String path = null;
+ String scriptName = null;
+ String cgiName = null;
+ String name = null;
- if (servletPath.startsWith(cginame)) {
- scriptname = contextPath + cginame;
- } else {
- scriptname = contextPath + servletPath + cginame;
+ path = context.getRealPath(cgiPath.toString());
+ if (path == null) {
+ /*
+ * The script doesn't exist directly on the file system. It
might be located in an archive or similar.
+ * Such scripts are extracted to the web application's
temporary file location.
+ */
+ File tmpCgiFile = new File(tmpDir + cgiPath.toString());
+ if (!tmpCgiFile.exists()) {
+
+ // Create directories
+ File parent = tmpCgiFile.getParentFile();
+ if (!parent.mkdirs() && !parent.isDirectory()) {
+
log.warn(sm.getString("cgiServlet.expandCreateDirFail",
parent.getAbsolutePath()));
+ return new String[] { null, null, null, null };
+ }
+
+ try (InputStream is =
context.getResourceAsStream(cgiPath.toString())) {
+ synchronized (expandFileLock) {
+ // Check if file was created by concurrent request
+ if (!tmpCgiFile.exists()) {
+ try {
+ Files.copy(is, tmpCgiFile.toPath());
+ } catch (IOException ioe) {
+
log.warn(sm.getString("cgiServlet.expandFail", cgiScriptURL,
+ tmpCgiFile.getAbsolutePath()),
ioe);
+ if (tmpCgiFile.exists()) {
+ if (!tmpCgiFile.delete()) {
+
log.warn(sm.getString("cgiServlet.expandDeleteFail",
+
tmpCgiFile.getAbsolutePath()));
+ }
+ }
+ return new String[] { null, null, null,
null };
+ }
+ if (log.isDebugEnabled()) {
+
log.debug(sm.getString("cgiServlet.expandOk", cgiScriptURL,
+ tmpCgiFile.getAbsolutePath()));
+ }
+ }
+ }
+ } catch (IOException ioe) {
+ log.warn(sm.getString("cgiServlet.expandCloseFail",
cgiScriptURL), ioe);
+ }
+ }
+ path = tmpCgiFile.getAbsolutePath();
}
+ scriptName = urlPath.toString();
+ cgiName = scriptName.substring(servletPath.length());
+ name = scriptName.substring(scriptName.lastIndexOf('/') + 1);
+
if (log.isTraceEnabled()) {
- log.trace(sm.getString("cgiServlet.find.found", name, path,
scriptname, cginame));
+ log.trace(sm.getString("cgiServlet.find.found", name, path,
scriptName, cgiName));
}
- return new String[] { path, scriptname, cginame, name };
+
+ return new String[] { path, scriptName, cgiName, name };
}
+
/**
* Constructs the CGI environment to be supplied to the invoked CGI
script; relies heavily on Servlet API
* methods and findCGI
@@ -895,13 +941,7 @@ public final class CGIServlet extends HttpServlet {
sPathInfoOrig = this.pathInfo;
sPathInfoOrig = sPathInfoOrig == null ? "" : sPathInfoOrig;
- if (webAppRootDir == null) {
- // The app has not been deployed in exploded form
- webAppRootDir = tmpDir.toString();
- expandCGIScript();
- }
-
- sCGINames = findCGI(sPathInfoOrig, webAppRootDir, contextPath,
servletPath, cgiPathPrefix);
+ sCGINames = findCGI(contextPath, servletPath, sPathInfoOrig,
cgiPathPrefix);
sCGIFullPath = sCGINames[0];
sCGIScriptName = sCGINames[1];
@@ -1027,93 +1067,6 @@ public final class CGIServlet extends HttpServlet {
return true;
}
- /**
- * Extracts requested resource from web app archive to context work
directory to enable CGI script to be
- * executed.
- */
- protected void expandCGIScript() {
I know it would have to be written a little differently, but why remove
this separate method? I think using the separate method makes the
calling code easier to read.
- StringBuilder srcPath = new StringBuilder();
- StringBuilder destPath = new StringBuilder();
- InputStream is = null;
-
- // paths depend on mapping
- if (cgiPathPrefix == null) {
- srcPath.append(pathInfo);
- is = context.getResourceAsStream(srcPath.toString());
- destPath.append(tmpDir);
- destPath.append(pathInfo);
- } else {
- // essentially same search algorithm as findCGI()
- srcPath.append(cgiPathPrefix);
- StringTokenizer pathWalker = new StringTokenizer(pathInfo,
"/");
- // start with first element
- while (pathWalker.hasMoreElements() && (is == null)) {
- srcPath.append('/');
- srcPath.append(pathWalker.nextElement());
- is = context.getResourceAsStream(srcPath.toString());
- }
- destPath.append(tmpDir);
- destPath.append('/');
- destPath.append(srcPath);
- }
-
- if (is == null) {
- // didn't find anything, give up now
- log.warn(sm.getString("cgiServlet.expandNotFound", srcPath));
- return;
- }
-
- try {
- File f = new File(destPath.toString());
- if (f.exists()) {
- // Don't need to expand if it already exists
- return;
- }
-
- // create directories
- File dir = f.getParentFile();
- if (!dir.mkdirs() && !dir.isDirectory()) {
- log.warn(sm.getString("cgiServlet.expandCreateDirFail",
dir.getAbsolutePath()));
- return;
- }
-
- try {
- synchronized (expandFileLock) {
- // make sure file doesn't exist
- if (f.exists()) {
- return;
- }
-
- // create file
- if (!f.createNewFile()) {
- return;
- }
-
- Files.copy(is, f.toPath());
-
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("cgiServlet.expandOk",
srcPath, destPath));
- }
- }
- } catch (IOException ioe) {
- log.warn(sm.getString("cgiServlet.expandFail", srcPath,
destPath), ioe);
- // delete in case file is corrupted
- if (f.exists()) {
- if (!f.delete()) {
-
log.warn(sm.getString("cgiServlet.expandDeleteFail", f.getAbsolutePath()));
- }
- }
- }
- } finally {
- try {
- is.close();
- } catch (IOException e) {
- log.warn(sm.getString("cgiServlet.expandCloseFail",
srcPath), e);
- }
- }
- }
-
-
/**
* Returns important CGI environment information in a multi-line text
format.
*
@@ -1409,9 +1362,9 @@ public final class CGIServlet extends HttpServlet {
* <LI><u>Allowed characters in path segments</u>: This
implementation does not allow non-terminal NULL segments
* in the path -- IOExceptions may be thrown;
* <LI><u>"<code>.</code>" and "<code>..</code>" path segments</u>:
This implementation does not allow
- * "<code>.</code>" and "<code>..</code>" in the path, and such
characters will result in an IOException
- * being thrown (this should never happen since Tomcat normalises the
requestURI before determining the
- * contextPath, servletPath and pathInfo);
+ * "<code>.</code>" and "<code>..</code>" in the path, and such
characters will result in an IOException being
+ * thrown (this should never happen since Tomcat normalises the
requestURI before determining the contextPath,
+ * servletPath and pathInfo);
* <LI><u>Implementation limitations</u>: This implementation does
not impose any limitations except as
* documented above. This implementation may be limited by the
servlet container used to house this
* implementation. In particular, all the primary CGI variable values
are derived either directly or indirectly
@@ -1449,7 +1402,7 @@ public final class CGIServlet extends HttpServlet {
Runtime rt;
BufferedReader cgiHeaderReader = null;
InputStream cgiOutput = null;
- BufferedReader commandsStdErr ;
+ BufferedReader commandsStdErr;
Thread errReaderThread = null;
BufferedOutputStream commandsStdIn;
Process proc = null;
diff --git a/java/org/apache/catalina/servlets/LocalStrings.properties
b/java/org/apache/catalina/servlets/LocalStrings.properties
index c0c92ea88d..28f7b87bac 100644
--- a/java/org/apache/catalina/servlets/LocalStrings.properties
+++ b/java/org/apache/catalina/servlets/LocalStrings.properties
@@ -21,7 +21,6 @@ cgiServlet.expandCloseFail=Failed to close input stream for
script at path [{0}]
cgiServlet.expandCreateDirFail=Failed to create destination directory [{0}]
for script expansion
cgiServlet.expandDeleteFail=Failed to delete file at [{0}] after IOException
during expansion
cgiServlet.expandFail=Failed to expand script at path [{0}] to [{1}]
-cgiServlet.expandNotFound=Unable to expand [{0}] as it could not be found
cgiServlet.expandOk=Expanded script at path [{0}] to [{1}]
cgiServlet.find.found=Found CGI: name [{0}], path [{1}], script name [{2}]
and CGI name [{3}]
cgiServlet.find.location=Looking for a file at [{0}]
diff --git a/java/org/apache/catalina/servlets/LocalStrings_fr.properties
b/java/org/apache/catalina/servlets/LocalStrings_fr.properties
index 3577389b2b..452e450620 100644
--- a/java/org/apache/catalina/servlets/LocalStrings_fr.properties
+++ b/java/org/apache/catalina/servlets/LocalStrings_fr.properties
@@ -21,7 +21,6 @@ cgiServlet.expandCloseFail=Impossible de fermer le flux
d''entrée du script ave
cgiServlet.expandCreateDirFail=Echec de la création du répertoire de
destination [{0}] pour la décompression du script
cgiServlet.expandDeleteFail=Impossible d''effacer le fichier [{0}] suite à
une IOException pendant la décompression
cgiServlet.expandFail=Impossible de faire l''expansion du script au chemin
[{0}] vers [{1}]
-cgiServlet.expandNotFound=Impossible de décompresser [{0}] car il n''a pas été
trouvé
cgiServlet.expandOk=Extrait le script du chemin [{0}] vers [{1}]
cgiServlet.find.found=Trouvé le CGI : nom [{0}], chemin [{1}], nom de script
[{2}] et nom du CGI [{3}]
cgiServlet.find.location=Recherche d''un fichier en [{0}]
diff --git a/java/org/apache/catalina/servlets/LocalStrings_ja.properties
b/java/org/apache/catalina/servlets/LocalStrings_ja.properties
index 91de715170..eacaccab42 100644
--- a/java/org/apache/catalina/servlets/LocalStrings_ja.properties
+++ b/java/org/apache/catalina/servlets/LocalStrings_ja.properties
@@ -21,7 +21,6 @@ cgiServlet.expandCloseFail=パス [{0}] のスクリプトの入力ストリー
cgiServlet.expandCreateDirFail=スクリプトの展開先ディレクトリ[{0}]の作成に失敗しました。
cgiServlet.expandDeleteFail=拡張中にIOExceptionの後に [{0}] でファイルを削除できませんでした
cgiServlet.expandFail=パス [{0}] のスクリプトを [{1}] に展開できませんでした
-cgiServlet.expandNotFound=見つけることができなかったため [{0}] を展開できません
cgiServlet.expandOk=パス [{0}] の [{1}] に展開されたスクリプト
cgiServlet.find.found=見つかったCGI: 名前 [{0}]、パス [{1}]、スクリプト名 [{2}]、CGI名 [{3}]
cgiServlet.find.location=ファイル [{0}] を探しています。
diff --git a/java/org/apache/catalina/servlets/LocalStrings_ko.properties
b/java/org/apache/catalina/servlets/LocalStrings_ko.properties
index aeb3d720d6..547c842722 100644
--- a/java/org/apache/catalina/servlets/LocalStrings_ko.properties
+++ b/java/org/apache/catalina/servlets/LocalStrings_ko.properties
@@ -21,7 +21,6 @@ cgiServlet.expandCloseFail=경로 [{0}]에 위치한 스크립트를 위한, 입
cgiServlet.expandCreateDirFail=스크립트를 압축해제 하기 위한 대상 디렉토리 [{0}]을(를) 생성하지 못했습니다.
cgiServlet.expandDeleteFail=압축해제 중 IOException이 발생한 후, [{0}]에 위치한 해당 파일을 삭제하지
못했습니다.
cgiServlet.expandFail=경로 [{0}]의 스크립트를 [{1}](으)로 압축해제 하지 못했습니다.
-cgiServlet.expandNotFound=[{0}]을(를) 찾을 수 없어서 압축해제 할 수 없습니다.
cgiServlet.expandOk=[{0}] 경로에 있는 스트립트가 [{1}](으)로 압축 해제되었습니다.
cgiServlet.find.found=CGI 발견: 이름 [{0}], 경로 [{1}], 스크립트 이름 [{2}], CGI 이름 [{3}]
cgiServlet.find.location=[{0}]에 위치한 파일을 찾는 중
diff --git a/java/org/apache/catalina/servlets/LocalStrings_zh_CN.properties
b/java/org/apache/catalina/servlets/LocalStrings_zh_CN.properties
index a2bdf51ba3..efce343f1f 100644
--- a/java/org/apache/catalina/servlets/LocalStrings_zh_CN.properties
+++ b/java/org/apache/catalina/servlets/LocalStrings_zh_CN.properties
@@ -21,7 +21,6 @@ cgiServlet.expandCloseFail=无法关闭路径[{0}]处脚本的输入流
cgiServlet.expandCreateDirFail=无法为脚本扩展创建目标目录[{0}]
cgiServlet.expandDeleteFail=扩展期间，发生IOException异常后删除文件[{0}]失败
cgiServlet.expandFail=在路径[{0}] 到[{1}] 展开脚本失败.
-cgiServlet.expandNotFound=无法展开[{0}]，因为找不到它。
cgiServlet.expandOk=从路径[{0}]到[{1}]扩展脚本
cgiServlet.find.found=找到CGI:name[{0}]、path[{1}]、script name[{2}]和CGI name[{3}]
cgiServlet.find.location=在 [{0}] 查找文件
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 44563fb95f..0b08c61b64 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -129,6 +129,10 @@
<pr>843</pr>: Fix off by one validation logic for partial PUT ranges
and associated test case. Submitted by Chenjp. (remm)
</fix>
+ <scode>
+ Refactor GCI servlet to access resources via the
+ <code>WebResource</code> API. (markt)
+ </scode>
</changelog>
</subsection>
<subsection name="Jasper">
-chris
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
