Skip to content

Commit

Permalink
fix: important code smells/forgery threats as of codeql results 2024-…
Browse files Browse the repository at this point in the history 
…05-16
  • Loading branch information
@drcgjung
drcgjung committed May 16, 2024
1 parent d49adbd commit 9c0c4eb
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 103 deletions.
10 changes: 10 additions & 0 deletions ...plane/agent-plane-protocol/src/main/java/org/eclipse/tractusx/agents/edc/AgentConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public class AgentConfig {
protected final Pattern serviceDenyPattern;
protected final Pattern serviceAssetAllowPattern;
protected final Pattern serviceAssetDenyPattern;
protected static final Pattern ASSET_REFERENCE_PATTERN = Pattern.compile("((?<url>[^#]+)#)?(?<asset>.+)");

/**
* references to EDC services
Expand Down Expand Up @@ -381,6 +382,15 @@ public Pattern getServiceAssetDenyPattern() {
return serviceAssetDenyPattern;
}

/**
* access
*
* @return regular expression for asset references
*/
public static Pattern getAssetReferencePattern() {
return ASSET_REFERENCE_PATTERN;
}

/**
* access
*
Expand Down
4 changes: 0 additions & 4 deletions ...ne/agent-plane-protocol/src/main/java/org/eclipse/tractusx/agents/edc/AgentExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import java.util.Map;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.regex.Pattern;

/**
* EDC extension that initializes the Agent subsystem (Agent Sources, Agent Endpoint and Federation Callbacks
Expand All @@ -65,9 +64,6 @@ public class AgentExtension implements ServiceExtension {
*/
protected static final String DEFAULT_CONTEXT_ALIAS = "default";
protected static final String CALLBACK_CONTEXT_ALIAS = "callback";
public static final Pattern GRAPH_PATTERN = Pattern.compile("((?<url>[^#]+)#)?(?<graph>.*Graph(Asset)?.*)");
public static final Pattern SKILL_PATTERN = Pattern.compile("((?<url>[^#]+)#)?(?<skill>.*Skill(Asset)?.*)");


/**
* dependency injection part
Expand Down
31 changes: 10 additions & 21 deletions ...-plane/agent-plane-protocol/src/main/java/org/eclipse/tractusx/agents/edc/SkillStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,12 @@
package org.eclipse.tractusx.agents.edc;

import java.util.Optional;
import java.util.regex.Matcher;

/**
* interface to a skill store
*/
public interface SkillStore {

/**
* match a given asset
*
* @param key asset name
* @return matcher
*/
static Matcher matchSkill(String key) {
return AgentExtension.SKILL_PATTERN.matcher(key);
}

/**
* check a given asset for being a skill
*
Expand All @@ -45,17 +34,17 @@ static Matcher matchSkill(String key) {
/**
* register a skill
*
* @param key asset name required
* @param skill query text required
* @param name of skill optional
* @param description of skill optional
* @param version of skill optional
* @param contract of skill optional
* @param dist of skill required
* @param isFederated whether skill maybe synchronized in catalogue
* @param key asset name required
* @param skill query text required
* @param name of skill optional
* @param description of skill optional
* @param version of skill optional
* @param contract of skill optional
* @param dist of skill required
* @param isFederated whether skill maybe synchronized in catalogue
* @param allowServicePattern regex for service to call in skill
* @param denyServicePattern regex for services denied in skill
* @param ontologies a set of ontologies
* @param denyServicePattern regex for services denied in skill
* @param ontologies a set of ontologies
* @return skill id
*/
String put(String key, String skill, String name, String description, String version, String contract, SkillDistribution dist, boolean isFederated, String allowServicePattern, String denyServicePattern, String... ontologies);
Expand Down
33 changes: 15 additions & 18 deletions ...nt-plane-protocol/src/main/java/org/eclipse/tractusx/agents/edc/http/AgentController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.apache.http.HttpStatus;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.tractusx.agents.edc.AgentConfig;
import org.eclipse.tractusx.agents.edc.AgentExtension;
import org.eclipse.tractusx.agents.edc.AgreementController;
import org.eclipse.tractusx.agents.edc.SkillDistribution;
import org.eclipse.tractusx.agents.edc.SkillStore;
Expand Down Expand Up @@ -63,6 +62,7 @@ public class AgentController {
protected final SparqlQueryProcessor processor;
protected final DelegationService delegationService;


/**
* creates a new agent controller
*
Expand Down Expand Up @@ -378,17 +378,15 @@ public Response executeQuery(String asset, HttpHeaders headers, HttpServletReque
String remoteUrl = null;

if (asset != null) {
Matcher matcher = AgentExtension.GRAPH_PATTERN.matcher(asset);
Matcher matcher = config.getAssetReferencePattern().matcher(asset);
if (matcher.matches()) {
remoteUrl = matcher.group("url");
graph = matcher.group("graph");
} else {
matcher = SkillStore.matchSkill(asset);
if (!matcher.matches()) {
return Response.status(Response.Status.BAD_REQUEST).build();
asset = matcher.group("asset");
if (asset.contains("Graph")) {
graph = asset;
} else if (asset.contains("Skill")) {
skill = asset;
}
remoteUrl = matcher.group("url");
skill = matcher.group("skill");
}
}

Expand All @@ -406,15 +404,14 @@ public Response executeQuery(String asset, HttpHeaders headers, HttpServletReque
}

try {
// exchange skill against text
if (asset != null) {
if (skillStore.isSkill(asset)) {
Optional<String> skillOption = skillStore.get(asset);
if (skillOption.isPresent()) {
skill = skillOption.get();
} else {
return HttpUtils.respond(monitor, headers, HttpStatus.SC_NOT_FOUND, "The requested skill is not registered.", null);
}
// exchange skill against text locally
if (asset != null && skill != null) {
Optional<String> skillOption = skillStore.get(skill);
if (skillOption.isPresent()) {
skill = skillOption.get();
} else {
skill = null;
return HttpUtils.respond(monitor, headers, HttpStatus.SC_NOT_FOUND, "The requested skill is not registered.", null);
}
}

Expand Down
113 changes: 57 additions & 56 deletions ...ane-protocol/src/main/java/org/eclipse/tractusx/agents/edc/http/transfer/AgentSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import org.eclipse.edc.connector.dataplane.spi.pipeline.StreamResult;
import org.eclipse.edc.http.spi.EdcHttpClient;
import org.eclipse.edc.spi.types.domain.transfer.DataFlowStartMessage;
import org.eclipse.tractusx.agents.edc.AgentExtension;
import org.eclipse.tractusx.agents.edc.AgentConfig;
import org.eclipse.tractusx.agents.edc.SkillDistribution;
import org.eclipse.tractusx.agents.edc.SkillStore;
import org.eclipse.tractusx.agents.edc.sparql.SparqlQueryProcessor;
Expand Down Expand Up @@ -70,7 +70,7 @@ public class AgentSource implements DataSource {
protected SkillStore skillStore;

protected DataFlowStartMessage request;

protected String matchmakingAgentUrl;

public static final String AGENT_BOUNDARY = "--";
Expand Down Expand Up @@ -102,35 +102,36 @@ protected StreamResult<Stream<Part>> openMatchmakingInternal() {
String graph = null;
String asset = String.valueOf(request.getSourceDataAddress().getProperties().get(AgentSourceHttpParamsDecorator.ASSET_PROP_ID));
if (asset != null && asset.length() > 0) {
Matcher graphMatcher = AgentExtension.GRAPH_PATTERN.matcher(asset);
if (graphMatcher.matches()) {
graph = asset;
}
Matcher skillMatcher = SkillStore.matchSkill(asset);
if (skillMatcher.matches()) {
var skillText = skillStore.get(asset);
if (skillText.isEmpty()) {
return StreamResult.error(format("Skill %s does not exist.", asset));
}
SkillDistribution distribution = skillStore.getDistribution(asset);
String params = request.getProperties().get(AgentSourceHttpParamsDecorator.QUERY_PARAMS);
SkillDistribution runMode = SkillDistribution.ALL;
if (params.contains("runMode=provider") || params.contains("runMode=PROVIDER")) {
runMode = SkillDistribution.PROVIDER;
} else if (params.contains("runMode=consumer") || params.contains("runMode=CONSUMER")) {
runMode = SkillDistribution.CONSUMER;
}
if (runMode == SkillDistribution.CONSUMER) {
if (distribution == SkillDistribution.PROVIDER) {
return StreamResult.error(String.format("Run distribution of skill %s should be consumer, but was set to provider only.", asset));
Matcher assetMatcher = AgentConfig.getAssetReferencePattern().matcher(asset);
if (assetMatcher.matches()) {
if (assetMatcher.group("asset").contains("Graph")) {
graph = asset;
} else if (assetMatcher.group("asset").contains("Skill")) {
var skillText = skillStore.get(asset);
if (skillText.isEmpty()) {
return StreamResult.error(format("Skill %s does not exist.", asset));
}
return StreamResult.success(Stream.of(new AgentPart("application/sparql-query", skillText.get().getBytes())));
} else if (runMode == SkillDistribution.PROVIDER && distribution == SkillDistribution.CONSUMER) {
return StreamResult.error(String.format("Run distribution of skill %s should be provider, but was set to consumer only.", asset));
SkillDistribution distribution = skillStore.getDistribution(asset);
String params = request.getProperties().get(AgentSourceHttpParamsDecorator.QUERY_PARAMS);
SkillDistribution runMode = SkillDistribution.ALL;
if (params.contains("runMode=provider") || params.contains("runMode=PROVIDER")) {
runMode = SkillDistribution.PROVIDER;
} else if (params.contains("runMode=consumer") || params.contains("runMode=CONSUMER")) {
runMode = SkillDistribution.CONSUMER;
}
if (runMode == SkillDistribution.CONSUMER) {
if (distribution == SkillDistribution.PROVIDER) {
return StreamResult.error(String.format("Run distribution of skill %s should be consumer, but was set to provider only.", asset));
}
return StreamResult.success(Stream.of(new AgentPart("application/sparql-query", skillText.get().getBytes())));
} else if (runMode == SkillDistribution.PROVIDER && distribution == SkillDistribution.CONSUMER) {
return StreamResult.error(String.format("Run distribution of skill %s should be provider, but was set to consumer only.", asset));
}
skill = skillText.get(); // default execution for runMode=ALL or runMode=provider and DistributionMode is ALL or provider
}
skill = skillText.get(); // default execution for runMode=ALL or runMode=provider and DistributionMode is ALL or provider
}
}

try (Response response = processor.execute(this.requestFactory.toRequest(params), skill, graph, request.getSourceDataAddress().getProperties())) {
if (!response.isSuccessful()) {
return StreamResult.error(format("Received code transferring HTTP data for request %s: %s - %s.", requestId, response.code(), response.message()));
Expand All @@ -147,7 +148,7 @@ protected StreamResult<Stream<Part>> openMatchmakingInternal() {
return StreamResult.error(e.getMessage());
}
}

/**
* executes a KA-MATCHMAKING REST API call and pipes the results into KA-TRANSFER
*
Expand All @@ -165,33 +166,33 @@ protected StreamResult<Stream<Part>> openMatchmakingRest() {

String url = baseUrl + "?asset=" + asset;
if (asset != null && asset.length() > 0) {
Matcher graphMatcher = AgentExtension.GRAPH_PATTERN.matcher(asset);
if (graphMatcher.matches()) {
graph = asset;
}
Matcher skillMatcher = SkillStore.matchSkill(asset);
if (skillMatcher.matches()) {
var skillText = skillStore.get(asset);
if (skillText.isEmpty()) {
return StreamResult.error(format("Skill %s does not exist.", asset));
}
SkillDistribution distribution = skillStore.getDistribution(asset);
String params = request.getProperties().get(AgentSourceHttpParamsDecorator.QUERY_PARAMS);
SkillDistribution runMode = SkillDistribution.ALL;
if (params.contains("runMode=provider") || params.contains("runMode=PROVIDER")) {
runMode = SkillDistribution.PROVIDER;
} else if (params.contains("runMode=consumer") || params.contains("runMode=CONSUMER")) {
runMode = SkillDistribution.CONSUMER;
}
if (runMode == SkillDistribution.CONSUMER) {
if (distribution == SkillDistribution.PROVIDER) {
return StreamResult.error(String.format("Run distribution of skill %s should be consumer, but was set to provider only.", asset));
Matcher assetMatcher = AgentConfig.getAssetReferencePattern().matcher(asset);
if (assetMatcher.matches()) {
if (assetMatcher.group("asset").contains("Graph")) {
graph = asset;
} else if (assetMatcher.group("asset").contains("Skill")) {
var skillText = skillStore.get(asset);
if (skillText.isEmpty()) {
return StreamResult.error(format("Skill %s does not exist.", asset));
}
SkillDistribution distribution = skillStore.getDistribution(asset);
String params = request.getProperties().get(AgentSourceHttpParamsDecorator.QUERY_PARAMS);
SkillDistribution runMode = SkillDistribution.ALL;
if (params.contains("runMode=provider") || params.contains("runMode=PROVIDER")) {
runMode = SkillDistribution.PROVIDER;
} else if (params.contains("runMode=consumer") || params.contains("runMode=CONSUMER")) {
runMode = SkillDistribution.CONSUMER;
}
return StreamResult.success(Stream.of(new AgentPart("application/sparql-query", skillText.get().getBytes())));
} else if (runMode == SkillDistribution.PROVIDER && distribution == SkillDistribution.CONSUMER) {
return StreamResult.error(String.format("Run distribution of skill %s should be provider, but was set to consumer only.", asset));
if (runMode == SkillDistribution.CONSUMER) {
if (distribution == SkillDistribution.PROVIDER) {
return StreamResult.error(String.format("Run distribution of skill %s should be consumer, but was set to provider only.", asset));
}
return StreamResult.success(Stream.of(new AgentPart("application/sparql-query", skillText.get().getBytes())));
} else if (runMode == SkillDistribution.PROVIDER && distribution == SkillDistribution.CONSUMER) {
return StreamResult.error(String.format("Run distribution of skill %s should be provider, but was set to consumer only.", asset));
}
skill = skillText.get(); // default execution for runMode=ALL or runMode=provider and DistributionMode is ALL or provider
}
skill = skillText.get(); // default execution for runMode=ALL or runMode=provider and DistributionMode is ALL or provider
}
}

Expand All @@ -202,7 +203,7 @@ protected StreamResult<Stream<Part>> openMatchmakingRest() {
} else {
assetValue = skill;
}

HttpUrl.Builder urlBuilder = HttpUrl.parse(url).newBuilder();
urlBuilder.addQueryParameter("asset", assetValue);
// Put parameters into request
Expand Down Expand Up @@ -294,12 +295,12 @@ public AgentSource.Builder request(DataFlowStartMessage request) {
dataSource.request = request;
return this;
}

public AgentSource.Builder matchmakingAgentUrl(String matchmakingAgentUrl) {
dataSource.matchmakingAgentUrl = matchmakingAgentUrl;
return this;
}

public AgentSource build() {
Objects.requireNonNull(dataSource.requestId, "requestId");
Objects.requireNonNull(dataSource.httpClient, "httpClient");
Expand Down
4 changes: 3 additions & 1 deletion ...t-plane-protocol/src/main/java/org/eclipse/tractusx/agents/edc/service/EdcSkillStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.io.IOException;
import java.util.List;
import java.util.Optional;
import java.util.regex.Matcher;

/**
* Implements a skill store based on EDC assets
Expand All @@ -47,7 +48,8 @@ public EdcSkillStore(DataManagement management, TypeManager typeManager, AgentCo

@Override
public boolean isSkill(String key) {
return SkillStore.matchSkill(key).matches();
Matcher matcher = config.getAssetReferencePattern().matcher(key);
return matcher.matches() && matcher.group("asset").contains("Skill");
}

@Override
Expand Down
11 changes: 9 additions & 2 deletions ...ne-protocol/src/main/java/org/eclipse/tractusx/agents/edc/service/InMemorySkillStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
// SPDX-License-Identifier: Apache-2.0
package org.eclipse.tractusx.agents.edc.service;

import org.eclipse.tractusx.agents.edc.AgentConfig;
import org.eclipse.tractusx.agents.edc.SkillDistribution;
import org.eclipse.tractusx.agents.edc.SkillStore;

import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.regex.Matcher;

/**
* An in-memory store for local skills
Expand All @@ -31,15 +33,20 @@ public class InMemorySkillStore implements SkillStore {
// temporary local skill store
protected final Map<String, String> skills = new HashMap<>();

protected AgentConfig config;

/**
* create the store
*/
public InMemorySkillStore() {
public InMemorySkillStore(AgentConfig config) {
this.config = config;
}


@Override
public boolean isSkill(String key) {
return SkillStore.matchSkill(key).matches();
Matcher matcher = config.getAssetReferencePattern().matcher(key);
return matcher.matches() && matcher.group("asset").contains("Skill");
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...lane-protocol/src/test/java/org/eclipse/tractusx/agents/edc/http/TestAgentController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public class TestAgentController extends RestControllerTestBase {


SparqlQueryProcessor processor=new SparqlQueryProcessor(serviceExecutorReg,monitor,agentConfig,store, typeManager);
InMemorySkillStore skillStore=new InMemorySkillStore();
InMemorySkillStore skillStore=new InMemorySkillStore(agentConfig);

DelegationServiceImpl delegationService=new DelegationServiceImpl(mockController,monitor,client,typeManager,agentConfig);
AgentController agentController=new AgentController(monitor,mockController,agentConfig,processor,skillStore,delegationService);
Expand Down

0 comments on commit 9c0c4eb

Please sign in to comment.