Skip to content

Fix issues in throttling policy import API #13560

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.

Sign up for GitHub

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

Merged
PasanT9 merged 1 commit into from
Jan 23, 2026
+21 −1
Merged

Fix issues in throttling policy import API #13560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...gt.impl/src/main/java/org/wso2/carbon/apimgt/impl/importexport/ImportExportConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public final class ImportExportConstants {

public static final String JSON_EXTENSION = ".json";
public static final String YAML_EXTENSION = ".yaml";
public static final String YML_EXTENSION = ".yml";

// Image resource
public static final String IMAGE_RESOURCE = "Image";
Expand Down
21 changes: 20 additions & 1 deletion ...src/main/java/org/wso2/carbon/apimgt/rest/api/admin/v1/impl/ThrottlingApiServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1225,13 +1225,32 @@ public Response exportThrottlingPolicy(String policyId, String policyName, Strin
*/
public static ExportThrottlePolicyDTO getImportedPolicy(InputStream uploadedInputStream, Attachment fileDetail)
throws ParseException, APIImportExportException, IOException {

Comment thread
PasanT9 marked this conversation as resolved.
File importFolder = CommonUtil.createTempDirectory(null);
String uploadFileName = fileDetail.getContentDisposition().getFilename();
Comment thread
PasanT9 marked this conversation as resolved.
String fileType = (uploadFileName.contains(ImportExportConstants.YAML_EXTENSION)) ?
if (StringUtils.isEmpty(uploadFileName)) {
throw new APIImportExportException("Invalid file name. File name cannot be null or empty.");
}
Comment on lines +1231 to +1233

Copilot AI Jan 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with other file upload handling in the codebase (e.g., RestApiAdminUtils.java:283), consider adding APIUtil.validateFileName(uploadFileName) before the path traversal check. This provides an additional layer of validation to reject file names containing obvious path traversal attempts like "../" or "..\".

Copilot uses AI. Check for mistakes.
// Validate file extension to prevent uploading unauthorized file types
String lowerCaseFileName = uploadFileName.toLowerCase();

Copilot AI Jan 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using toLowerCase() without specifying a Locale can lead to unexpected behavior in certain locales (e.g., Turkish locale where 'I'.toLowerCase() becomes 'ı' instead of 'i'). For security-sensitive file extension validation, it's recommended to use toLowerCase(Locale.ROOT) or toLowerCase(Locale.ENGLISH) to ensure consistent behavior across all locales.

Copilot uses AI. Check for mistakes.
boolean isYamlFile =
lowerCaseFileName.endsWith(ImportExportConstants.YAML_EXTENSION) || lowerCaseFileName.endsWith(
ImportExportConstants.YML_EXTENSION);
boolean isJsonFile = lowerCaseFileName.endsWith(ImportExportConstants.JSON_EXTENSION);
if (!isYamlFile && !isJsonFile) {
throw new APIImportExportException("Invalid file type. Only YAML and JSON files are allowed.");
}
String fileType = isYamlFile ?
ImportExportConstants.EXPORT_POLICY_TYPE_YAML :
ImportExportConstants.EXPORT_POLICY_TYPE_JSON;
// Validating the canonical path
String absolutePath = importFolder.getAbsolutePath() + File.separator + uploadFileName;
File targetFile = new File(absolutePath);
String canonicalPath = targetFile.getCanonicalPath();
String canonicalImportPath = importFolder.getCanonicalPath();
if (!canonicalPath.startsWith(canonicalImportPath + File.separator)) {
throw new APIImportExportException("Invalid file name.");

Copilot AI Jan 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message should be more descriptive about why the file name is invalid. Consider changing it to something like "Invalid file name. File path is outside the target directory." This is more consistent with similar error messages in the codebase (e.g., RestApiAdminUtils.java:288-290) and helps with debugging.

Suggested change
throw new APIImportExportException("Invalid file name.");
throw new APIImportExportException("Invalid file name. File path is outside the target directory.");

Copilot uses AI. Check for mistakes.
}
FileUtils.copyInputStreamToFile(uploadedInputStream, targetFile);
return preprocessImportedArtifact(absolutePath, fileType);
}
Comment on lines 1226 to 1256

Copilot AI Jan 23, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The security enhancements in this method (file name validation, file type validation, and path traversal protection) should be covered by tests to ensure they work correctly and prevent regressions. Consider adding tests that verify: 1) null/empty file names are rejected, 2) only YAML/YML/JSON extensions are accepted, 3) path traversal attempts are blocked, 4) valid files are accepted.

Copilot uses AI. Check for mistakes.
Expand Down
Loading
⚡