Skip to content
This repository has been archived by the owner on May 1, 2023. It is now read-only.

Commit

Permalink
Reversible Trace ID sanitization
Browse files Browse the repository at this point in the history
Summary:
# Problem Description

Loom uses base64 encoding for the string representation of Trace IDs. The base64 alphabet uses `+` and `/` characters in addition to alphanumeric characters. Trace file names contain Trace IDs, and in order it to be valid filenames, Trace IDs are sanitized to replace unsupported characters.

The current approach is replacing both `+` and `/` with an underscore (`_`). This transformation makes it impossible to reconstruct the actual Trace ID from the filename, which could be useful in situations where we need to access the Trace ID without parsing the trace file contents (on either client or server).

# Change Description

With this change, the sanitization function becomes reversible, by having it replace `+` with `_p_` and `/` with `_s_`.

Reviewed By: aandreyeu

Differential Revision: D36764316

fbshipit-source-id: 744ac37fe6c190436a2445170fac6d1003b3b2f1
  • Loading branch information
elliekorn authored and facebook-github-bot committed Jun 1, 2022
1 parent 256981b commit 67c41c4
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 11 deletions.
24 changes: 21 additions & 3 deletions java/main/com/facebook/profilo/core/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ CONSTANTS = [
"ProfiloConstants.java",
]

UTIL = [
"TraceIdSanitizer.java",
]

EVENTS = [
"TraceEvents.java",
]
Expand Down Expand Up @@ -33,7 +37,7 @@ LISTENER = [
"TraceOrchestratorListener.java",
]

TRACE_WRITER_LISTENTER = [
TRACE_WRITER_LISTENER = [
"TraceWriterListener.java",
]

Expand Down Expand Up @@ -67,6 +71,18 @@ fb_core_android_library(
],
)

fb_core_android_library(
name = "util",
srcs = UTIL,
labels = ["supermodule:android/default/loom.core"],
tests = [
"//fbandroid/libraries/profilo/java/test/com/facebook/profilo/core:core",
],
visibility = [
"PUBLIC",
],
)

fb_core_android_library(
name = "events",
srcs = EVENTS,
Expand Down Expand Up @@ -103,6 +119,7 @@ fb_core_android_library(
":constants",
":listener",
":registry",
":util",
profilo_path("java/main/com/facebook/profilo/mmapbuf/core:core"),
profilo_path("java/main/com/facebook/profilo/writer:writer"),
profilo_path("deps/fbtrace:utils"),
Expand Down Expand Up @@ -137,7 +154,7 @@ fb_core_android_library(

fb_core_android_library(
name = "writer_listener",
srcs = TRACE_WRITER_LISTENTER,
srcs = TRACE_WRITER_LISTENER,
labels = ["supermodule:android/default/loom.core"],
visibility = [
profilo_path("..."),
Expand Down Expand Up @@ -169,7 +186,7 @@ fb_core_android_library(
name = "core",
srcs = glob(
["*.java"],
exclude = CONSTANTS + EVENTS + CONTROL + REGISTRY + LISTENER + TRACE_WRITER_LISTENTER + UPLOAD,
exclude = CONSTANTS + UTIL + EVENTS + CONTROL + REGISTRY + LISTENER + TRACE_WRITER_LISTENER + UPLOAD,
),
labels = ["supermodule:android/default/loom.core"],
tests = [
Expand Down Expand Up @@ -204,6 +221,7 @@ fb_core_android_library(
":listener",
":registry",
":upload",
":util",
":writer_listener",
profilo_path("java/main/com/facebook/profilo/config:config"),
profilo_path("java/main/com/facebook/profilo/logger:logger"),
Expand Down
3 changes: 1 addition & 2 deletions java/main/com/facebook/profilo/core/TraceControl.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import com.facebook.profilo.logger.Trace;
import com.facebook.profilo.mmapbuf.core.Buffer;
import com.facebook.profilo.mmapbuf.core.MmapBufferManager;
import com.facebook.profilo.writer.NativeTraceWriter;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
Expand Down Expand Up @@ -289,7 +288,7 @@ public List<TraceContext> getCurrentTraces() {
}

public File getTraceFolder(String trace_id) {
return new File(mFolder, NativeTraceWriter.getSanitizedTraceFolderName(trace_id));
return new File(mFolder, TraceIdSanitizer.sanitizeTraceId(trace_id));
}

public boolean startTrace(int controller, int flags, @Nullable Object context, long longContext) {
Expand Down
48 changes: 48 additions & 0 deletions java/main/com/facebook/profilo/core/TraceIdSanitizer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* Copyright 2004-present, Facebook, Inc.
*
* <p>Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file
* except in compliance with the License. You may obtain a copy of the License at
*
* <p>http://www.apache.org/licenses/LICENSE-2.0
*
* <p>Unless required by applicable law or agreed to in writing, software distributed under the
* License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.facebook.profilo.core;

/**
* Provides operations to convert between base64-encoded Trace IDs and file/folder names.
*
* <p>This class is not meant to be instantiated.
*
* <p>The transformation is designed to be reversible for diagnostic purposes.
*/
public final class TraceIdSanitizer {
private static final String SANITIZED_PLUS = "_p_";
private static final String SANITIZED_SLASH = "_s_";

/**
* Converts the Trace ID into a string that can safely be used as part of a directory name.
*
* <p>Base64 uses alphanumeric characters as well as '+' and '/'. In filenames, we replace '+'
* with '_p_' and '/' with '_s_'.
*
* @param traceId The base64-encoded Trace ID to convert.
* @return A string consisting only of alphanumeric characters and '_'.
*/
public static String sanitizeTraceId(String traceId) {
return traceId.replace("+", SANITIZED_PLUS).replace("/", SANITIZED_SLASH);
}

/**
* Returns the original Trace ID corresponding to the given sanitized Trace ID.
*
* @seealso sanitizeTraceId
*/
public static String restoreSanitizedTraceId(String sanitizedTraceId) {
return sanitizedTraceId.replace(SANITIZED_PLUS, "+").replace(SANITIZED_SLASH, "/");
}
}
3 changes: 1 addition & 2 deletions java/main/com/facebook/profilo/core/TraceOrchestrator.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import com.facebook.profilo.logger.Trace;
import com.facebook.profilo.mmapbuf.core.Buffer;
import com.facebook.profilo.mmapbuf.core.MmapBufferManager;
import com.facebook.profilo.writer.NativeTraceWriter;
import java.io.File;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
Expand Down Expand Up @@ -352,7 +351,7 @@ public File getExtraDataFile(TraceContext context, BaseTraceProvider provider) {
synchronized (this) {
mainFolder = mFileManager.getFolder();
}
String sanitizedTraceID = NativeTraceWriter.getSanitizedTraceFolderName(context.encodedTraceId);
String sanitizedTraceID = TraceIdSanitizer.sanitizeTraceId(context.encodedTraceId);
File extraFolder = new File(new File(mainFolder, sanitizedTraceID), EXTRA_DATA_FOLDER_NAME);
if (!extraFolder.isDirectory() && !extraFolder.mkdirs()) {
// If the main & other process try to call mkdirs() simultaneously, one of the mkdirs will
Expand Down
4 changes: 0 additions & 4 deletions java/main/com/facebook/profilo/writer/NativeTraceWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,6 @@ public final class NativeTraceWriter {
SoLoader.loadLibrary("profilo");
}

public static String getSanitizedTraceFolderName(String trace_id) {
return trace_id.replaceAll("[^a-zA-Z0-9\\-_.]", "_");
}

@DoNotStrip private HybridData mHybridData;

public NativeTraceWriter(
Expand Down
48 changes: 48 additions & 0 deletions java/test/com/facebook/profilo/core/TraceIdSanitizerTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* Copyright 2004-present, Facebook, Inc.
*
* <p>Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file
* except in compliance with the License. You may obtain a copy of the License at
*
* <p>http://www.apache.org/licenses/LICENSE-2.0
*
* <p>Unless required by applicable law or agreed to in writing, software distributed under the
* License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.facebook.profilo.core;

import static com.facebook.profilo.core.TraceIdSanitizer.restoreSanitizedTraceId;
import static com.facebook.profilo.core.TraceIdSanitizer.sanitizeTraceId;
import static org.assertj.core.api.Assertions.assertThat;

import com.facebook.testing.robolectric.v4.WithTestDefaultsRunner;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;

/** Tests {@link TraceIdSanitizer} */
@RunWith(WithTestDefaultsRunner.class)
public class TraceIdSanitizerTest {
@Before
public void setUp() {}

@Test
public void testSanitizeTraceId() {
assertThat(sanitizeTraceId("")).isEqualTo("");
assertThat(sanitizeTraceId("+")).isEqualTo("_p_");
assertThat(sanitizeTraceId("/")).isEqualTo("_s_");
assertThat(sanitizeTraceId("aAmMzZ059")).isEqualTo("aAmMzZ059");
assertThat(sanitizeTraceId("a+b/c")).isEqualTo("a_p_b_s_c");
}

@Test
public void testRestoreSanitizedTraceId() {
assertThat(restoreSanitizedTraceId("")).isEqualTo("");
assertThat(restoreSanitizedTraceId("_p_")).isEqualTo("+");
assertThat(restoreSanitizedTraceId("_s_")).isEqualTo("/");
assertThat(restoreSanitizedTraceId("aAmMzZ059")).isEqualTo("aAmMzZ059");
assertThat(restoreSanitizedTraceId("a_p_b_s_c")).isEqualTo("a+b/c");
}
}

0 comments on commit 67c41c4

Please sign in to comment.