fix pr comments

Author: djankows

modules/swagger-codegen/src/main/java/io/swagger/codegen/AbstractGenerator.java

@@ -22,13 +22,12 @@ public abstract class AbstractGenerator {
     private static final Logger LOGGER = LoggerFactory.getLogger(AbstractGenerator.class);
 
     @SuppressWarnings("static-method")
-    public File writeToFile(String filename, String contents) throws IOException, SecurityException {
+    public File writeToFile(String filename, String contents) throws IOException {
         LOGGER.info("writing file " + filename);
         SecureFileUtils.validatePath(filename);
         File output = new File(filename);
 
         if (output.getParent() != null && !new File(output.getParent()).exists()) {
-            SecureFileUtils.validatePath(output.getParent());
             File parent = new File(output.getParent());
             parent.mkdirs();
         }
@@ -109,7 +108,7 @@ public String getFullTemplateFile(CodegenConfig config, String templateFile) {
                 return embeddedLibTemplateFile;
             }
         }
-            
+
         // Fall back to the template file embedded/packaged in the JAR file...
         return config.embeddedTemplateDir() + File.separator + templateFile;
     }

modules/swagger-codegen/src/main/java/io/swagger/codegen/DefaultCodegen.java

@@ -3563,11 +3563,7 @@ public String apiTestFilename(String templateName, String tag) {
     }
 
     public boolean shouldOverwrite(String filename) {
-        try {
-            SecureFileUtils.validatePath(filename);
-        } catch (SecurityException e) {
-            return true;
-        }
+        SecureFileUtils.validatePath(filename);
         return !(skipOverwrite && new File(filename).exists());
     }
 
@@ -3831,17 +3827,12 @@ public void writeOptional(String outputFolder, SupportingFile supportingFile) {
         else {
             folder = supportingFile.destinationFilename;
         }
-        try {
-            SecureFileUtils.validatePath(folder);
-            if(!new File(folder).exists()) {
-                supportingFiles.add(supportingFile);
-            } else {
-                LOGGER.info("Skipped overwriting " + supportingFile.destinationFilename + " as the file already exists in " + folder);
-            }
-        } catch (SecurityException e) {
-            LOGGER.error("Error while validating path" + folder, e);
+        SecureFileUtils.validatePath(folder);
+        if(!new File(folder).exists()) {
+            supportingFiles.add(supportingFile);
+        } else {
+            LOGGER.info("Skipped overwriting " + supportingFile.destinationFilename + " as the file already exists in " + folder);
         }
-
     }
 
     /**

modules/swagger-codegen/src/main/java/io/swagger/codegen/ignore/CodegenIgnoreProcessor.java

@@ -41,19 +41,14 @@ public CodegenIgnoreProcessor(final String baseDirectory) {
      */
     @SuppressWarnings("WeakerAccess")
     public CodegenIgnoreProcessor(final String baseDirectory, final String ignoreFile) {
-        try {
-            SecureFileUtils.validatePath(baseDirectory);
-            final File directory = new File(baseDirectory);
-            SecureFileUtils.validatePath(directory);
-            SecureFileUtils.validatePath(ignoreFile);
-            final File targetIgnoreFile = new File(directory, ignoreFile);
-            if (directory.exists() && directory.isDirectory()) {
-                loadFromFile(targetIgnoreFile);
-            } else {
-                LOGGER.warn("Output directory does not exist, or is inaccessible. No file (.swagger-codegen-ignore) will be evaluated.");
-            }
-        } catch (SecurityException e) {
-            LOGGER.error("Security violation: attempted to access unsafe ignore file path ", e);
+        SecureFileUtils.validatePath(baseDirectory);
+        final File directory = new File(baseDirectory);
+        SecureFileUtils.validatePath(ignoreFile);
+        final File targetIgnoreFile = new File(directory, ignoreFile);
+        if (directory.exists() && directory.isDirectory()) {
+            loadFromFile(targetIgnoreFile);
+        } else {
+            LOGGER.warn("Output directory does not exist, or is inaccessible. No file (.swagger-codegen-ignore) will be evaluated.");
         }
     }
 
@@ -63,12 +58,8 @@ public CodegenIgnoreProcessor(final String baseDirectory, final String ignoreFil
      * @param targetIgnoreFile The ignore file location.
      */
     public CodegenIgnoreProcessor(final File targetIgnoreFile) {
-        try {
-            SecureFileUtils.validatePath(targetIgnoreFile);
-            loadFromFile(targetIgnoreFile);
-        } catch (SecurityException e) {
-            LOGGER.error("Security violation: attempted to access unsafe ignore file path: " + targetIgnoreFile.getAbsolutePath(), e);
-        }
+        SecureFileUtils.validatePath(targetIgnoreFile);
+        loadFromFile(targetIgnoreFile);
     }
 
     private void loadFromFile(File targetIgnoreFile) {

modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/JavaJAXRSSpecServerCodegen.java

@@ -200,7 +200,7 @@ public void preprocessSwagger(Swagger swagger) {
             String swaggerJson = Json.pretty(swagger);
             SecureFileUtils.validatePath(outputFolder);
             FileUtils.writeStringToFile(new File(outputFolder + File.separator + "swagger.json"), swaggerJson, StandardCharsets.UTF_8);
-        } catch (IOException | SecurityException e) {
+        } catch (IOException e) {
             throw new RuntimeException(e.getMessage(), e.getCause());
         }
         super.preprocessSwagger(swagger);

modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/RubyClientCodegen.java

@@ -327,7 +327,7 @@ public String generateGemName(String moduleName) {
     }
 
     @Override
-    public String escapeReservedWord(String name) {           
+    public String escapeReservedWord(String name) {
         if(this.reservedWordsMappings().containsKey(name)) {
             return this.reservedWordsMappings().get(name);
         }
@@ -742,13 +742,8 @@ public void setGemAuthorEmail(String gemAuthorEmail) {
     @Override
     public boolean shouldOverwrite(String filename) {
         // skip spec file as the file might have been updated with new test cases
-        try {
-            SecureFileUtils.validatePath(filename);
-            return !(skipOverwrite && new File(filename).exists());
-        } catch (SecurityException e) {
-            LOGGER.warn("Security violation: attempted to check unsafe file path: " + filename, e);
-            return false;
-        }
+        SecureFileUtils.validatePath(filename);
+        return !(skipOverwrite && new File(filename).exists());
         //
         //return super.shouldOverwrite(filename) && !filename.endsWith("_spec.rb");
     }

modules/swagger-codegen/src/main/java/io/swagger/codegen/utils/SecureFileUtils.java

@@ -10,7 +10,11 @@
  */
 public class SecureFileUtils {
 
-    public static void validatePath(File file) throws SecurityException {
+    private SecureFileUtils() {
+        // Utility class
+    }
+
+    public static void validatePath(File file) {
         if (file == null) {
             throw new IllegalArgumentException("File cannot be null");
         }
@@ -32,7 +36,7 @@ public static void validatePath(File file) throws SecurityException {
         }
     }
 
-    public static void validatePath(String path) throws SecurityException {
+    public static void validatePath(String path) {
         if (path == null || path.trim().isEmpty()) {
             throw new IllegalArgumentException("Path cannot be null or empty");
         }

modules/swagger-codegen/src/test/java/io/swagger/codegen/DefaultCodegenTest.java

@@ -37,8 +37,10 @@ public void testAdditionalPropertiesPutForConfigValues() throws Exception {
     @Test
     public void testShouldOverwriteWithPathTraversal() {
         DefaultCodegen codegen = new DefaultCodegen();
-        boolean result = codegen.shouldOverwrite("../../../etc/passwd");
-
-        Assert.assertTrue(result, "shouldOverwrite should return false when SecurityException is thrown");
+        Assert.assertThrows(
+            "shouldOverwrite should throw SecurityException for suspicious path",
+            SecurityException.class,
+            () -> codegen.shouldOverwrite("../../../etc/passwd")
+        );
     }
 }

modules/swagger-codegen/src/test/java/io/swagger/codegen/ignore/CodegenIgnoreProcessorSecurityTest.java

@@ -9,20 +9,21 @@ public class CodegenIgnoreProcessorSecurityTest {
 
     @Test
     public void testConstructorWithPathTraversal() {
-        CodegenIgnoreProcessor processor = new CodegenIgnoreProcessor("../../../etc");
-
-        Assert.assertNotNull(processor, "Processor should be created despite security violation");
-        Assert.assertTrue(processor.getExclusionRules().isEmpty(), "No exclusion rules should be loaded");
-        Assert.assertTrue(processor.getInclusionRules().isEmpty(), "No inclusion rules should be loaded");
+        Assert.assertThrows(
+                "shouldOverwrite should throw SecurityException for suspicious path",
+                SecurityException.class,
+                () -> new CodegenIgnoreProcessor("../../../etc")
+        );
     }
 
     @Test
     public void testFileConstructorWithPathTraversal() {
         File maliciousFile = new File("../../../etc/passwd");
-        CodegenIgnoreProcessor processor = new CodegenIgnoreProcessor(maliciousFile);
 
-        Assert.assertNotNull(processor, "Processor should be created despite security violation");
-        Assert.assertTrue(processor.getExclusionRules().isEmpty(), "No exclusion rules should be loaded");
-        Assert.assertTrue(processor.getInclusionRules().isEmpty(), "No inclusion rules should be loaded");
+        Assert.assertThrows(
+                "shouldOverwrite should throw SecurityException for suspicious path",
+                SecurityException.class,
+                () -> new CodegenIgnoreProcessor(maliciousFile)
+        );
     }
 }

modules/swagger-codegen/src/test/java/io/swagger/codegen/languages/RubyClientCodegenTest.java

@@ -8,9 +8,10 @@ public class RubyClientCodegenTest {
     @Test
     public void testShouldOverwriteWithPathTraversal() {
         RubyClientCodegen codegen = new RubyClientCodegen();
-
-        boolean result = codegen.shouldOverwrite("../../../etc/passwd");
-
-        Assert.assertFalse(result, "shouldOverwrite should return false when SecurityException is thrown");
+        Assert.assertThrows(
+                "shouldOverwrite should throw SecurityException for suspicious path",
+                SecurityException.class,
+                () -> codegen.shouldOverwrite("../../../etc/passwd")
+        );
     }
 }

modules/swagger-generator/src/main/java/io/swagger/generator/resource/SwaggerResource.java

@@ -60,12 +60,11 @@ public Response downloadFile(@PathParam("fileId") String fileId) throws Exceptio
         System.out.println("looking for fileId " + fileId);
         System.out.println("got filename " + g.getFilename());
         if (g.getFilename() != null) {
-//            SecureFileUtils.validatePath(g.getFilename());
+            SecureFileUtils.validatePath(g.getFilename());
             File file = new java.io.File(g.getFilename());
             byte[] bytes = org.apache.commons.io.FileUtils.readFileToByteArray(file);
 
             try {
-//                SecureFileUtils.validatePath(file.getParentFile());
                 FileUtils.deleteDirectory(file.getParentFile());
             } catch (Exception e) {
                 System.out.println("failed to delete file " + file.getAbsolutePath());