From be757ec5a671fd2bbef22917076774aff8f22141 Mon Sep 17 00:00:00 2001
From: Igor Vinokur <ivinokur@redhat.com>
Date: Tue, 14 Mar 2023 12:03:39 +0200
Subject: [PATCH] Check the devfile content before creating a factory (#463)

When Che creates a workspace from a repository URL, first we iterate and find a related git provider handler, then we gat a devfile location according to the handler rules. If we pass an unsupported git repository url we will have a default handler which will try find a devfile by the same url as the repository. In that case we will have an html content as a devfile.

We need to check the content before parsing the devfile, and throw a specific error in case if the content is not yaml.
---
 .../server/urlfactory/URLFactoryBuilder.java    |  8 +++++++-
 .../urlfactory/URLFactoryBuilderTest.java       | 17 +++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java
index 098500b15d..648d769780 100644
--- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java
+++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilder.java
@@ -135,9 +135,15 @@ public class URLFactoryBuilder {
       if (isNullOrEmpty(devfileYamlContent)) {
         return Optional.empty();
       }
-
       try {
         JsonNode parsedDevfile = devfileParser.parseYamlRaw(devfileYamlContent);
+        // We might have an html content in the parsed devfile, in case if the access is restricted,
+        // or if the URL points to a wrong resource.
+        try {
+          devfileVersionDetector.devfileVersion(parsedDevfile);
+        } catch (DevfileException e) {
+          throw new ApiException("Failed to fetch devfile");
+        }
         return Optional.of(
             createFactory(parsedDevfile, overrideProperties, fileContentProvider, location));
       } catch (OverrideParameterException e) {
diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java
index 36cd5966ad..bc24344372 100644
--- a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java
+++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/urlfactory/URLFactoryBuilderTest.java
@@ -21,6 +21,7 @@ import static org.eclipse.che.dto.server.DtoFactory.newDto;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyMap;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.testng.Assert.assertEquals;
@@ -444,6 +445,22 @@ public class URLFactoryBuilderTest {
     }
   }
 
+  @Test(
+      expectedExceptions = ApiException.class,
+      expectedExceptionsMessageRegExp = "Failed to fetch devfile")
+  public void shouldThrowErrorOnUnsupportedDevfileContent()
+      throws ApiException, DevfileException, IOException {
+    JsonNode jsonNode = mock(JsonNode.class);
+    when(fileContentProvider.fetchContent(eq("location"))).thenReturn("unsupported content");
+    when(devfileParser.parseYamlRaw(eq("unsupported content"))).thenReturn(jsonNode);
+    when(devfileVersionDetector.devfileVersion(eq(jsonNode))).thenThrow(new DevfileException(""));
+    urlFactoryBuilder.createFactoryFromDevfile(
+        new DefaultFactoryUrl().withDevfileFileLocation("location"),
+        fileContentProvider,
+        emptyMap(),
+        false);
+  }
+
   @DataProvider
   public static Object[][] devfileExceptions() {
     return new Object[][] {