Skip to content

Commit 8b63e25

Browse files
vinokurigvinokurig
authored
Reduce the RAW url resolver to yaml content check only (#803) (#804)
Remove the RAW url pattern check from the RAW devfile url resolver, to have only the yaml content check. This is needed to revert the resolver priority to highest, and do not accept urls like https://github.com/vinokurig/test/blob/master/path/devfile.yaml. See #793
1 parent 495c697 commit 8b63e25

File tree

4 files changed

+44
-17
lines changed

4 files changed

+44
-17
lines changed

‎.ci/openshift-ci/test-azure-no-pat-oauth-flow-raw-devfile-url.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ trap "catchFinish" EXIT SIGINT
2929

3030
setupTestEnvironment ${OCP_NON_ADMIN_USER_NAME}
3131
testFactoryResolverResponse ${PUBLIC_REPO_RAW_PATH_URL} 200
32-
testFactoryResolverResponse ${PRIVATE_REPO_RAW_PATH_URL} 500
32+
testFactoryResolverResponse ${PRIVATE_REPO_RAW_PATH_URL} 400
3333

3434
testCloneGitRepoNoProjectExists ${PUBLIC_REPO_WORKSPACE_NAME} ${PUBLIC_PROJECT_NAME} ${PUBLIC_REPO_RAW_PATH_URL} ${USER_CHE_NAMESPACE}
3535
deleteTestWorkspace ${PUBLIC_REPO_WORKSPACE_NAME} ${USER_CHE_NAMESPACE}

‎assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#
2-
# Copyright (c) 2012-2024 Red Hat, Inc.
2+
# Copyright (c) 2012-2025 Red Hat, Inc.
33
# This program and the accompanying materials are made
44
# available under the terms of the Eclipse Public License 2.0
55
# which is available at https://www.eclipse.org/legal/epl-2.0/

‎wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/RawDevfileUrlFactoryParameterResolver.java‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
import static com.google.common.base.Strings.isNullOrEmpty;
1515
import static java.lang.String.format;
16-
import static org.eclipse.che.api.factory.server.FactoryResolverPriority.LOWEST;
16+
import static org.eclipse.che.api.factory.server.FactoryResolverPriority.HIGHEST;
1717
import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME;
1818

1919
import com.fasterxml.jackson.databind.JsonNode;
@@ -24,7 +24,7 @@
2424
import java.net.URISyntaxException;
2525
import java.net.URL;
2626
import java.util.Map;
27-
import java.util.regex.Pattern;
27+
import java.util.Optional;
2828
import javax.inject.Inject;
2929
import org.eclipse.che.api.core.ApiException;
3030
import org.eclipse.che.api.core.BadRequestException;
@@ -35,7 +35,7 @@
3535
import org.eclipse.che.api.workspace.server.devfile.DevfileParser;
3636
import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
3737
import org.eclipse.che.api.workspace.server.devfile.URLFileContentProvider;
38-
import org.eclipse.che.api.workspace.server.devfile.exception.DevfileFormatException;
38+
import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException;
3939

4040
/**
4141
* {@link FactoryParametersResolver} implementation to resolve factory based on url parameter as a
@@ -45,8 +45,6 @@ public class RawDevfileUrlFactoryParameterResolver extends BaseFactoryParameterR
4545
implements FactoryParametersResolver {
4646

4747
private static final String PROVIDER_NAME = "raw-devfile-url";
48-
private static final Pattern PATTERN =
49-
Pattern.compile("^https?://.*\\.ya?ml((\\?token=.*)|(\\?at=refs/heads/.*))?$");
5048

5149
protected final URLFactoryBuilder urlFactoryBuilder;
5250
protected final URLFetcher urlFetcher;
@@ -71,15 +69,18 @@ public RawDevfileUrlFactoryParameterResolver(
7169
@Override
7270
public boolean accept(Map<String, String> factoryParameters) {
7371
String url = factoryParameters.get(URL_PARAMETER_NAME);
74-
return !isNullOrEmpty(url) && (PATTERN.matcher(url).matches() || containsYaml(url));
72+
return !isNullOrEmpty(url) && containsYaml(url);
7573
}
7674

7775
private boolean containsYaml(String requestURL) {
7876
try {
79-
String fetch = urlFetcher.fetch(requestURL);
77+
Optional<String> credentials = new DefaultFactoryUrl().withUrl(requestURL).getCredentials();
78+
URLFileContentProvider urlFileContentProvider =
79+
new URLFileContentProvider(new URL(requestURL).toURI(), urlFetcher);
80+
String fetch = urlFileContentProvider.fetchContent(requestURL, credentials.orElse(null));
8081
JsonNode parsedYaml = devfileParser.parseYamlRaw(fetch);
8182
return !parsedYaml.isEmpty();
82-
} catch (IOException | DevfileFormatException e) {
83+
} catch (IOException | URISyntaxException | DevfileException e) {
8384
return false;
8485
}
8586
}
@@ -128,6 +129,6 @@ public RemoteFactoryUrl parseFactoryUrl(String factoryUrl) throws ApiException {
128129

129130
@Override
130131
public FactoryResolverPriority priority() {
131-
return LOWEST;
132+
return HIGHEST;
132133
}
133134
}

‎wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/RawDevfileUrlFactoryParameterResolverTest.java‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2012-2024 Red Hat, Inc.
2+
* Copyright (c) 2012-2025 Red Hat, Inc.
33
* This program and the accompanying materials are made
44
* available under the terms of the Eclipse Public License 2.0
55
* which is available at https://www.eclipse.org/legal/epl-2.0/
@@ -27,6 +27,7 @@
2727

2828
import com.fasterxml.jackson.databind.JsonNode;
2929
import java.io.FileNotFoundException;
30+
import java.util.Base64;
3031
import java.util.HashMap;
3132
import java.util.Map;
3233
import org.eclipse.che.api.core.BadRequestException;
@@ -111,7 +112,13 @@ public void shouldThrowExceptionOnInvalidURL(String url, String message) throws
111112
}
112113

113114
@Test(dataProvider = "devfileUrls")
114-
public void shouldAcceptRawDevfileUrl(String url) {
115+
public void shouldAcceptRawDevfileUrl(String url) throws Exception {
116+
// given
117+
JsonNode jsonNode = mock(JsonNode.class);
118+
when(urlFetcher.fetch(eq(url), eq(null))).thenReturn(DEVFILE);
119+
when(devfileParser.parseYamlRaw(eq(DEVFILE))).thenReturn(jsonNode);
120+
when(jsonNode.isEmpty()).thenReturn(false);
121+
115122
// when
116123
boolean result =
117124
rawDevfileUrlFactoryParameterResolver.accept(singletonMap(URL_PARAMETER_NAME, url));
@@ -124,7 +131,7 @@ public void shouldAcceptRawDevfileUrl(String url) {
124131
public void shouldAcceptRawDevfileUrlWithoutExtension(String url) throws Exception {
125132
// given
126133
JsonNode jsonNode = mock(JsonNode.class);
127-
when(urlFetcher.fetch(eq(url))).thenReturn(DEVFILE);
134+
when(urlFetcher.fetch(eq(url), eq(null))).thenReturn(DEVFILE);
128135
when(devfileParser.parseYamlRaw(eq(DEVFILE))).thenReturn(jsonNode);
129136
when(jsonNode.isEmpty()).thenReturn(false);
130137

@@ -141,7 +148,26 @@ public void shouldAcceptRawDevfileUrlWithYaml() throws Exception {
141148
// given
142149
JsonNode jsonNode = mock(JsonNode.class);
143150
String url = "https://host/path/devfile";
144-
when(urlFetcher.fetch(eq(url))).thenReturn(DEVFILE);
151+
when(urlFetcher.fetch(eq(url), eq(null))).thenReturn(DEVFILE);
152+
when(devfileParser.parseYamlRaw(eq(DEVFILE))).thenReturn(jsonNode);
153+
when(jsonNode.isEmpty()).thenReturn(false);
154+
155+
// when
156+
boolean result =
157+
rawDevfileUrlFactoryParameterResolver.accept(singletonMap(URL_PARAMETER_NAME, url));
158+
159+
// then
160+
assertTrue(result);
161+
}
162+
163+
@Test
164+
public void shouldAcceptRawDevfileUrlWithCredentials() throws Exception {
165+
// given
166+
JsonNode jsonNode = mock(JsonNode.class);
167+
String url = "https://credentials@host/path/devfile";
168+
when(urlFetcher.fetch(
169+
eq(url), eq("Basic " + Base64.getEncoder().encodeToString("credentials:".getBytes()))))
170+
.thenReturn(DEVFILE);
145171
when(devfileParser.parseYamlRaw(eq(DEVFILE))).thenReturn(jsonNode);
146172
when(jsonNode.isEmpty()).thenReturn(false);
147173

@@ -158,7 +184,7 @@ public void shouldNotAcceptPublicGitRepositoryUrl() throws Exception {
158184
// given
159185
JsonNode jsonNode = mock(JsonNode.class);
160186
String gitRepositoryUrl = "https://host/user/repo.git";
161-
when(urlFetcher.fetch(eq(gitRepositoryUrl))).thenReturn("unsupported content");
187+
when(urlFetcher.fetch(eq(gitRepositoryUrl), eq(null))).thenReturn("unsupported content");
162188
when(devfileParser.parseYamlRaw(eq("unsupported content"))).thenReturn(jsonNode);
163189
when(jsonNode.isEmpty()).thenReturn(true);
164190

@@ -175,7 +201,7 @@ public void shouldNotAcceptPublicGitRepositoryUrl() throws Exception {
175201
public void shouldNotAcceptPrivateGitRepositoryUrl() throws Exception {
176202
// given
177203
String gitRepositoryUrl = "https://host/user/private-repo.git";
178-
when(urlFetcher.fetch(eq(gitRepositoryUrl))).thenThrow(new FileNotFoundException());
204+
when(urlFetcher.fetch(eq(gitRepositoryUrl), eq(null))).thenThrow(new FileNotFoundException());
179205

180206
// when
181207
boolean result =

0 commit comments

Comments
 (0)