Skip to content

Commit

Permalink
update default session-id-header for tags-http-exporter (#1620)
Browse files Browse the repository at this point in the history
* replace default session-id-header

* update tests

* update OpenAPI docu

* update docu

* refactor browser propagation

* add access-control-max-age

* add note about tomcat parameter
  • Loading branch information
EddeCCC authored Oct 9, 2023
1 parent 61429e4 commit a4e425a
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,6 @@ inspectit:
# Additional limitations: key-length -> 128, value-length -> 2048, attribute-count -> 128
session-limit: 100
# header, which will be read during browser-propagation to receive the session-ID
session-id-header: "Cookie"
session-id-header: "Session-Id"
# how long the data should be stored in the server in seconds
time-to-live: 300
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ protected boolean doEnable(InspectitConfig configuration) {
protected boolean doDisable() {
if(server != null) {
try {
log.info("Stopping Tags HTTP-Server");
log.info("Stopping Tags HTTP-Server - All sessions will be removed");
server.stop();
sessionStorage.clearDataStorages();
sessionStorage.setExporterActive(false);
Expand All @@ -101,7 +101,7 @@ protected boolean startServer(String host, int port, String path, HttpServlet se
log.info("Starting Tags HTTP-Server on {}:{}{} ", host, port, path);
server.start();
} catch (Exception e) {
log.warn("Starting of Tags HTTP-Server failed");
log.error("Starting of Tags HTTP-Server failed", e);
return false;
}
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,11 @@ public class BrowserPropagationServlet extends HttpServlet {

/**
* Header, which should be used to store the session-Ids
* Default-key: "Cookie"
*/
private final String sessionIdHeader;

/**
* List of allowed Orgins, which are able to access the HTTP-server
* List of allowed Origins, which are able to access the HTTP-server
*/
private final List<String> allowedOrigins;
private final ObjectMapper mapper;
Expand All @@ -54,7 +53,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro

//If wildcard is used, allow every origin
//Alternatively, check if current origin is allowed
if(this.allowedOrigins.contains("*") || this.allowedOrigins.contains(origin)) {
if(allowedOrigins.contains("*") || allowedOrigins.contains(origin)) {
response.setHeader("Access-Control-Allow-Origin", origin);
response.setHeader("Access-Control-Allow-Methods", "GET");
response.setHeader("Access-Control-Allow-Credentials", "true");
Expand Down Expand Up @@ -91,7 +90,7 @@ protected void doPut(HttpServletRequest request, HttpServletResponse response) {

//If wildcard is used, allow every origin
//Alternatively, check if current origin is allowed
if(this.allowedOrigins.contains("*") || this.allowedOrigins.contains(origin)) {
if(allowedOrigins.contains("*") || allowedOrigins.contains(origin)) {
response.setHeader("Access-Control-Allow-Origin", origin);
response.setHeader("Access-Control-Allow-Methods", "PUT");
response.setHeader("Access-Control-Allow-Credentials", "true");
Expand Down Expand Up @@ -127,13 +126,20 @@ protected void doOptions(HttpServletRequest request, HttpServletResponse respons
log.debug("Tags HTTP-server received OPTIONS-request");
String origin = request.getHeader("Origin");
String accessControlRequestMethod = request.getHeader("Access-Control-Request-Method");

if (origin != null && accessControlRequestMethod != null &&
(this.allowedOrigins.contains("*") || this.allowedOrigins.contains(origin))
String accessControlRequestHeaders = request.getHeader("Access-Control-Request-Headers");

if (
origin != null &&
accessControlRequestMethod != null &&
accessControlRequestHeaders != null &&
(allowedOrigins.contains("*") || allowedOrigins.contains(origin)) &&
accessControlRequestHeaders.equalsIgnoreCase(sessionIdHeader)
) {
response.setHeader("Access-Control-Allow-Origin", origin);
response.setHeader("Access-Control-Allow-Methods", "GET, PUT");
response.setHeader("Access-Control-Allow-Headers", sessionIdHeader);
response.setHeader("Access-Control-Allow-Credentials", "true");
response.setHeader("Access-Control-Max-Age", "3600");
response.setStatus(HttpServletResponse.SC_OK);
}
else response.setStatus(HttpServletResponse.SC_FORBIDDEN);
Expand All @@ -145,7 +151,7 @@ private Map<String, String> getRequestBody(HttpServletRequest request) {
return entrySet.stream()
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
} catch (Exception e) {
log.info("Request to Tags HTTP-server failed");
log.warn("Request to Tags HTTP-server failed", e);
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public BrowserPropagationDataStorage() {
public void writeData(Map<String, ?> newPropagationData) {
Map <String, Object> validatedData = validateEntries(newPropagationData);

if(!validateAttributeLength(validatedData)) {
if(!validateAttributeCount(validatedData)) {
log.warn("Unable to write data: Data count limit was exceeded");
return;
}
Expand All @@ -42,6 +42,10 @@ public Map<String, Object> readData() {
return new HashMap<>(propagationData);
}

public int getStorageSize() {
return propagationData.size();
}

public void updateTimestamp(long newTimestamp) {
latestTimestamp = newTimestamp;
}
Expand All @@ -55,7 +59,7 @@ public long calculateElapsedTime(long currentTime) {
return currentTime - latestTimestamp;
}

private boolean validateAttributeLength(Map<String, ?> newPropagationData) {
private boolean validateAttributeCount(Map<String, ?> newPropagationData) {
Set<String> keySet = new HashSet<>(propagationData.keySet());
keySet.retainAll(newPropagationData.keySet());
//Add size of both maps and subtract the common keys
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,14 @@ public void cleanUpData(int timeToLive) {
long currentTime = System.currentTimeMillis();
dataStorages.forEach((id, storage) -> {
long elapsedTime = storage.calculateElapsedTime(currentTime) / 1000;
if(timeToLive < elapsedTime) dataStorages.remove(id);
if(timeToLive < elapsedTime) {
dataStorages.remove(id);
log.debug("Time to Live expired for the following session: " + id);
}
else {
int storageSize = storage.getStorageSize();
log.debug("There are " + storageSize + " data entries stored in session: " + id);
}
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
/**
* Class to regulate the currently used session-id-key.
* The session-id-key is used to extract session-ids from http-request-headers to allow browser propagation.
* The session-id-key can change during runtime and needs to updated inside the PROPAGATION_FIELDS in ContextPropagationUtil.
* The session-id-key can change during runtime and needs to be updated inside the PROPAGATION_FIELDS in ContextPropagationUtil.
*/
@Slf4j
@Component
Expand All @@ -24,7 +24,7 @@ public class BrowserPropagationUtil {
@Autowired
private InspectitEnvironment env;
@Getter
private static String sessionIdHeader = "Cookie";
private static String sessionIdHeader = "Session-Id";

@PostConstruct
public void initialize() {
Expand All @@ -40,7 +40,7 @@ private void configEventListener(InspectitConfigChangedEvent event) {
String oldSessionIdHeader = event.getOldConfig().getExporters().getTags().getHttp().getSessionIdHeader();
String newSessionIdHeader = event.getNewConfig().getExporters().getTags().getHttp().getSessionIdHeader();

if(!oldSessionIdHeader.equals(newSessionIdHeader)) ContextPropagationUtil.setSessionIdHeader(newSessionIdHeader);
if(!oldSessionIdHeader.equals(newSessionIdHeader)) setSessionIdHeader(newSessionIdHeader);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public class BrowserPropagationHttpExporterServiceIntTest extends SpringTestBase
private static String url;

private static final String path = "/inspectit";
private static final String sessionIDHeader = "Cookie";
private static final String sessionIDHeader = "Session-Id";
private static final String allowedOrigin = "localhost";

@BeforeEach
Expand Down Expand Up @@ -248,6 +248,7 @@ void verifyOptionsEndpointSuccessfulGet() throws IOException {
HttpOptions optionsRequest = new HttpOptions(url);
optionsRequest.setHeader("Origin", allowedOrigin);
optionsRequest.setHeader("access-control-request-method", "GET");
optionsRequest.setHeader("access-control-request-headers", sessionIDHeader);

CloseableHttpResponse response = testClient.execute(optionsRequest);
int statusCode = response.getStatusLine().getStatusCode();
Expand All @@ -261,6 +262,7 @@ void verifyOptionsEndpointSuccessfulPut() throws IOException {
HttpOptions optionsRequest = new HttpOptions(url);
optionsRequest.setHeader("Origin", allowedOrigin);
optionsRequest.setHeader("access-control-request-method", "PUT");
optionsRequest.setHeader("access-control-request-headers", sessionIDHeader);

CloseableHttpResponse response = testClient.execute(optionsRequest);
int statusCode = response.getStatusLine().getStatusCode();
Expand All @@ -270,7 +272,7 @@ void verifyOptionsEndpointSuccessfulPut() throws IOException {
}

@Test
void verifyOptionsEndpointWithMissingHeader() throws IOException {
void verifyOptionsEndpointWithMissingHeaders() throws IOException {
HttpOptions optionsRequest = new HttpOptions(url);
optionsRequest.setHeader("Origin", allowedOrigin);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class BrowserPropagationDataStorageTest extends SpringTestBase {

Map<String, String> headers;

private static final String sessionIdHeader = "Cookie";
private static final String sessionIdHeader = "Session-Id";
private static final String sessionId = "test=83311527d6a6de76a60a72a041808a63;b0b2b4cf=ad9fef38-4942-453a-9243-7d8422803604";

@BeforeEach
Expand Down Expand Up @@ -64,7 +64,7 @@ void verifyNoDataHasBeenWritten() {
assertThat(dataStorage.readData()).isEmpty();

ctx.close();
assertThat(dataStorage.readData()).isEmpty();
assertThat(dataStorage.getStorageSize()).isZero();
assertThat(ContextUtil.currentInspectitContext()).isNull();
}

Expand Down Expand Up @@ -135,7 +135,7 @@ void verifyAttributeCountLimit() {
ctx.close();
assertThat(dataStorage.readData()).doesNotContainEntry("key1", "value321");
assertThat(dataStorage.readData()).doesNotContainEntry("keyABC", "valueABC");
assertThat(dataStorage.readData().size()).isEqualTo(128);
assertThat(dataStorage.getStorageSize()).isEqualTo(128);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public class BrowserPropagationUtilTest {
@InjectMocks
BrowserPropagationUtil browserPropagationUtil;

final static String key = "Cookie";
final static String key = "Session-Id";

@BeforeEach
void setUp() {
Expand Down
Loading

0 comments on commit a4e425a

Please sign in to comment.