diff --git a/.github/workflows/Semgrep.yml b/.github/workflows/Semgrep.yml index 95c5710..7f44a29 100644 --- a/.github/workflows/Semgrep.yml +++ b/.github/workflows/Semgrep.yml @@ -34,6 +34,8 @@ jobs: steps: # Fetch project source with GitHub Actions Checkout. - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + with: + persist-credentials: false # Run the "semgrep ci" command on the command line of the docker image. - run: semgrep ci --sarif --output=semgrep.sarif env: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7e19e91..4403b45 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -23,7 +23,7 @@ jobs: if: ${{ github.event_name == 'workflow_dispatch' }} with: text: ${{ github.event.inputs.branch }} - regex: '^[a-zA-Z0-9_/\-]+$' + regex: '^[A-Za-z0-9][A-Za-z0-9._/-]*$' - name: Break on invalid branch name run: exit 1 if: ${{ github.event_name == 'workflow_dispatch' && steps.regex-match.outputs && steps.regex-match.outputs.match == '' }} @@ -35,7 +35,7 @@ jobs: java-version: ${{ matrix.java }} - name: Get yarn cache directory path id: yarn-cache-dir-path - run: echo "::set-output name=dir::$(yarn cache dir)" + run: echo "dir=$(yarn cache dir)" >> "$GITHUB_OUTPUT" - uses: actions/cache@6f8efc29b200d32929f49075959781ed54ec270c # v3 with: path: ${{ steps.yarn-cache-dir-path.outputs.dir }} @@ -47,6 +47,10 @@ jobs: env: CLI_BRANCH: ${{ github.event.inputs.branch }} run: | + if ! [[ "$CLI_BRANCH" =~ ^[A-Za-z0-9][A-Za-z0-9._/-]*$ ]] || [[ "$CLI_BRANCH" == *..* ]]; then + echo "Invalid branch name" >&2 + exit 1 + fi cd /tmp git clone --branch "$CLI_BRANCH" --depth 1 https://github.com/percy/cli cd cli @@ -55,7 +59,7 @@ jobs: yarn yarn build yarn global:link - cd ${{ github.workspace }} + cd "$GITHUB_WORKSPACE" yarn remove @percy/cli && yarn link `echo $PERCY_PACKAGES` npx percy --version - run: MOZ_HEADLESS=1 npx percy exec -- mvn test diff --git a/README.md b/README.md index e2eca62..8e12741 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,13 @@ The snapshot method arguments: - `sync` - For getting syncronous results https://www.browserstack.com/docs/percy/advanced/sync-comparison-results - `responsiveSnapshotCapture` - For capturing snapshot of responsive websites +### Cookies + +By default the SDK sends the browser's cookies to the Percy CLI so that pages and assets behind a +logged-in session load during asset discovery. To stop sending them, set +`PERCY_DO_NOT_USE_CAPTURED_COOKIES=true`. With this set, assets that need a session may fail to load +and snapshots can render incorrectly; the SDK logs a warning once when the option is active. + ## Upgrading diff --git a/src/main/java/io/percy/selenium/Percy.java b/src/main/java/io/percy/selenium/Percy.java index 5607898..135bd0b 100644 --- a/src/main/java/io/percy/selenium/Percy.java +++ b/src/main/java/io/percy/selenium/Percy.java @@ -18,6 +18,7 @@ import java.time.Duration; import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nullable; @@ -55,6 +56,12 @@ public class Percy { private static boolean PERCY_RESPONSIVE_CAPTURE_RELOAD_PAGE = Boolean.parseBoolean(System.getenv().getOrDefault("PERCY_RESPONSIVE_CAPTURE_RELOAD_PAGE", "false").toLowerCase()); + // Opt-out of forwarding browser cookies to the CLI (same knob the CLI honours) + private static boolean PERCY_DO_NOT_USE_CAPTURED_COOKIES = "true".equalsIgnoreCase( + System.getenv().getOrDefault("PERCY_DO_NOT_USE_CAPTURED_COOKIES", "false").trim()); + + private static final AtomicBoolean cookieOptOutWarned = new AtomicBoolean(false); + private static boolean PERCY_RESPONSIVE_CAPTURE_MIN_HEIGHT = Boolean.parseBoolean(System.getenv().getOrDefault("PERCY_RESPONSIVE_CAPTURE_MIN_HEIGHT", "false").toLowerCase()); private static final int WIDTHS_CONFIG_TIMEOUT_MS = 30000; // for logging @@ -461,12 +468,7 @@ public JSONObject snapshot(String name, Map options) { try { JavascriptExecutor jse = (JavascriptExecutor) driver; jse.executeScript(fetchPercyDOM()); - Set cookies = new HashSet<>(); - try { - cookies = driver.manage().getCookies(); - } catch(Exception e) { - log("Cookie collection failed " + e.getMessage(), "debug"); - } + Set cookies = collectCookies(); if (isCaptureResponsiveDOM(mergedOptions)) { domSnapshot = captureResponsiveDom(driver, cookies, mergedOptions); } else { @@ -709,6 +711,35 @@ private String buildSnapshotJS(Map options) { return jsBuilder.toString(); } + /** + * Collects the browser cookies the CLI replays during asset discovery. + * Returns an empty set when PERCY_DO_NOT_USE_CAPTURED_COOKIES=true, and + * never throws. + */ + private Set collectCookies() { + if (isCookieForwardingDisabled()) { + return new HashSet<>(); + } + try { + return driver.manage().getCookies(); + } catch (Exception e) { + log("Cookie collection failed " + e.getMessage(), "debug"); + return new HashSet<>(); + } + } + + private static boolean isCookieForwardingDisabled() { + if (!PERCY_DO_NOT_USE_CAPTURED_COOKIES) { + return false; + } + if (cookieOptOutWarned.compareAndSet(false, true)) { + log("Not sending browser cookies to Percy because PERCY_DO_NOT_USE_CAPTURED_COOKIES is set. " + + "Pages or assets that need a logged-in session may fail to load during asset discovery, " + + "so snapshots can render incorrectly.", "warn"); + } + return true; + } + /** * Readiness gate: runs PercyDOM.waitForReady BEFORE serialize. * @@ -747,15 +778,17 @@ protected Object waitForReady(JavascriptExecutor jse, Map option } } try { + // The config is passed as a script argument rather than spliced + // into the script source, so its values can never become code. String script = - "var cfg = " + readinessConfig.toString() + ";" + "var cfg = arguments[0];" + "var done = arguments[arguments.length - 1];" + "try {" + " if (typeof PercyDOM !== 'undefined' && typeof PercyDOM.waitForReady === 'function') {" + " PercyDOM.waitForReady(cfg).then(function(r){ done(r); }).catch(function(){ done(); });" + " } else { done(); }" + "} catch (e) { done(); }"; - return jse.executeAsyncScript(script); + return jse.executeAsyncScript(script, readinessConfig.toMap()); } catch (Exception e) { log("waitForReady failed, proceeding to serialize: " + e.getMessage(), "debug"); return null; @@ -847,6 +880,11 @@ private Map processFrame(WebElement frameElement, Map) jse.executeScript( "return PercyDOM.serialize(" + optionsJson.toString() + ")" ); + // PercyDOM.serialize captures the frame's document.cookie; drop it when opted out. + if (iframeSnapshot != null && isCookieForwardingDisabled()) { + iframeSnapshot = new HashMap<>(iframeSnapshot); + iframeSnapshot.remove("cookies"); + } } catch (Exception e) { log("Failed to process cross-origin frame " + finalFrameUrl + ": " + e.getMessage(), "error"); throw new RuntimeException("Failed to process cross-origin frame " + finalFrameUrl, e); @@ -876,7 +914,7 @@ Map getSerializedDOM(JavascriptExecutor jse, Set cookies Map domSnapshot = (Map) jse.executeScript(buildSnapshotJS(options)); Map mutableSnapshot = new HashMap<>(domSnapshot); - mutableSnapshot.put("cookies", cookies); + mutableSnapshot.put("cookies", isCookieForwardingDisabled() ? Collections.emptySet() : cookies); // Attach readiness diagnostics so the CLI can log timing and pass/fail if (readinessDiagnostics != null) { diff --git a/src/main/java/io/percy/selenium/cucumber/PercySteps.java b/src/main/java/io/percy/selenium/cucumber/PercySteps.java index ce396e3..5b73856 100644 --- a/src/main/java/io/percy/selenium/cucumber/PercySteps.java +++ b/src/main/java/io/percy/selenium/cucumber/PercySteps.java @@ -59,12 +59,52 @@ */ public class PercySteps { - private static WebDriver driver; - private static Percy percy; - private static List> regions = new ArrayList<>(); + // Driver/Percy pair. Each thread that calls setDriver gets its own pair so + // scenarios running in parallel don't overwrite each other. The most recent + // pair is also kept as a shared fallback for threads that never called + // setDriver themselves (e.g. a driver set once for the whole run). + private static final class Session { + final WebDriver driver; + final Percy percy; + + Session(WebDriver driver, Percy percy) { + this.driver = driver; + this.percy = percy; + } + } + + private static final ThreadLocal threadSession = new ThreadLocal<>(); + private static volatile Session sharedSession; + private static final ThreadLocal>> threadRegions = + ThreadLocal.withInitial(ArrayList::new); private static final String CUCUMBER_CLIENT_NAME = "percy-cucumber-java-selenium"; + private static Session currentSession() { + Session session = threadSession.get(); + return session != null ? session : sharedSession; + } + + private static void setSession(Session session) { + threadSession.set(session); + sharedSession = session; + } + + private static WebDriver currentDriver() { + Session session = currentSession(); + return session != null ? session.driver : null; + } + + private static Percy percy() { + Session session = currentSession(); + return session != null ? session.percy : null; + } + + // Package-private seam so tests can swap in a mock Percy for the current thread. + static void setPercy(Percy percy) { + setSession(new Session(currentDriver(), percy)); + } + /** * Set the WebDriver instance for Percy to use. * Call this from your Cucumber hooks before using any Percy steps. @@ -72,8 +112,8 @@ public class PercySteps { * @param webDriver the Selenium WebDriver instance */ public static void setDriver(WebDriver webDriver) { - driver = webDriver; - percy = new Percy(driver); + Percy percy = new Percy(webDriver); + setSession(new Session(webDriver, percy)); // Identify as Cucumber wrapper in Percy build info String sdkVersion = Percy.getSdkVersion(); @@ -116,7 +156,7 @@ static String resolveCucumberVersion(java.util.concurrent.Callable resol * @return the Percy instance, or null if not initialized */ public static Percy getPercy() { - return percy; + return percy(); } /** @@ -124,9 +164,9 @@ public static Percy getPercy() { * Call this from your Cucumber hooks in teardown. */ public static void reset() { - percy = null; - driver = null; - regions.clear(); + threadSession.remove(); + sharedSession = null; + threadRegions.remove(); } // ------------------------------------------------------------------ @@ -135,12 +175,13 @@ public static void reset() { @Given("I have a Percy instance") public void iHaveAPercyInstance() { + WebDriver driver = currentDriver(); if (driver == null) { throw new IllegalStateException( "WebDriver not set. Call PercySteps.setDriver(driver) in your @Before hook."); } - if (percy == null) { - percy = new Percy(driver); + if (percy() == null) { + setSession(new Session(driver, new Percy(driver))); } } @@ -149,7 +190,7 @@ public void iCreateIgnoreRegionCSS(String cssSelector) { Map params = new HashMap<>(); params.put("algorithm", "ignore"); params.put("elementCSS", cssSelector); - regions.add(percy.createRegion(params)); + regions().add(percy().createRegion(params)); } @Given("I create a Percy ignore region with XPath {string}") @@ -157,7 +198,7 @@ public void iCreateIgnoreRegionXPath(String xpath) { Map params = new HashMap<>(); params.put("algorithm", "ignore"); params.put("elementXpath", xpath); - regions.add(percy.createRegion(params)); + regions().add(percy().createRegion(params)); } @Given("I create a Percy ignore region with bounding box {int}, {int}, {int}, {int}") @@ -171,7 +212,7 @@ public void iCreateIgnoreRegionBoundingBox(int x, int y, int width, int height) Map params = new HashMap<>(); params.put("algorithm", "ignore"); params.put("boundingBox", boundingBox); - regions.add(percy.createRegion(params)); + regions().add(percy().createRegion(params)); } @Given("I create a Percy consider region with CSS selector {string}") @@ -179,7 +220,7 @@ public void iCreateConsiderRegionCSS(String cssSelector) { Map params = new HashMap<>(); params.put("algorithm", "standard"); params.put("elementCSS", cssSelector); - regions.add(percy.createRegion(params)); + regions().add(percy().createRegion(params)); } @Given("I create a Percy consider region with CSS selector {string} and diff sensitivity {int}") @@ -188,7 +229,7 @@ public void iCreateConsiderRegionCSSWithSensitivity(String cssSelector, int sens params.put("algorithm", "standard"); params.put("elementCSS", cssSelector); params.put("diffSensitivity", sensitivity); - regions.add(percy.createRegion(params)); + regions().add(percy().createRegion(params)); } @Given("I create a Percy intelliignore region with CSS selector {string}") @@ -196,7 +237,7 @@ public void iCreateIntelliIgnoreRegionCSS(String cssSelector) { Map params = new HashMap<>(); params.put("algorithm", "intelliignore"); params.put("elementCSS", cssSelector); - regions.add(percy.createRegion(params)); + regions().add(percy().createRegion(params)); } @Given("I create a Percy ignore region with CSS selector {string} and padding {int}") @@ -205,7 +246,7 @@ public void iCreateIgnoreRegionCSSWithPadding(String cssSelector, int padding) { params.put("algorithm", "ignore"); params.put("elementCSS", cssSelector); params.put("padding", padding); - regions.add(percy.createRegion(params)); + regions().add(percy().createRegion(params)); } @Given("I create a Percy ignore region with XPath {string} and padding {int}") @@ -214,7 +255,7 @@ public void iCreateIgnoreRegionXPathWithPadding(String xpath, int padding) { params.put("algorithm", "ignore"); params.put("elementXpath", xpath); params.put("padding", padding); - regions.add(percy.createRegion(params)); + regions().add(percy().createRegion(params)); } @Given("I create a Percy consider region with XPath {string}") @@ -222,7 +263,7 @@ public void iCreateConsiderRegionXPath(String xpath) { Map params = new HashMap<>(); params.put("algorithm", "standard"); params.put("elementXpath", xpath); - regions.add(percy.createRegion(params)); + regions().add(percy().createRegion(params)); } @Given("I create a Percy consider region with XPath {string} and diff sensitivity {int}") @@ -231,7 +272,7 @@ public void iCreateConsiderRegionXPathWithSensitivity(String xpath, int sensitiv params.put("algorithm", "standard"); params.put("elementXpath", xpath); params.put("diffSensitivity", sensitivity); - regions.add(percy.createRegion(params)); + regions().add(percy().createRegion(params)); } @Given("I create a Percy intelliignore region with XPath {string}") @@ -239,12 +280,12 @@ public void iCreateIntelliIgnoreRegionXPath(String xpath) { Map params = new HashMap<>(); params.put("algorithm", "intelliignore"); params.put("elementXpath", xpath); - regions.add(percy.createRegion(params)); + regions().add(percy().createRegion(params)); } @Given("I clear Percy regions") public void iClearPercyRegions() { - regions.clear(); + regions().clear(); } // ------------------------------------------------------------------ @@ -253,115 +294,118 @@ public void iClearPercyRegions() { @When("I take a Percy snapshot named {string}") public void iTakeSnapshot(String name) { - percy.snapshot(name); + percy().snapshot(name); } @When("I take a Percy snapshot named {string} with widths {string}") public void iTakeSnapshotWithWidths(String name, String widths) { Map options = new HashMap<>(); options.put("widths", parseWidths(widths)); - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with min height {int}") public void iTakeSnapshotWithMinHeight(String name, int minHeight) { Map options = new HashMap<>(); options.put("minHeight", minHeight); - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with Percy CSS {string}") public void iTakeSnapshotWithCSS(String name, String percyCSS) { Map options = new HashMap<>(); options.put("percyCSS", percyCSS); - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with scope {string}") public void iTakeSnapshotWithScope(String name, String scope) { Map options = new HashMap<>(); options.put("scope", scope); - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with layout mode") public void iTakeSnapshotWithLayout(String name) { Map options = new HashMap<>(); options.put("enableLayout", true); - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with JavaScript enabled") public void iTakeSnapshotWithJS(String name) { Map options = new HashMap<>(); options.put("enableJavaScript", true); - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with labels {string}") public void iTakeSnapshotWithLabels(String name, String labels) { Map options = new HashMap<>(); options.put("labels", labels); - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with test case {string}") public void iTakeSnapshotWithTestCase(String name, String testCase) { Map options = new HashMap<>(); options.put("testCase", testCase); - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with Shadow DOM disabled") public void iTakeSnapshotWithShadowDomDisabled(String name) { Map options = new HashMap<>(); options.put("disableShadowDom", true); - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with responsive capture") public void iTakeSnapshotWithResponsiveCapture(String name) { Map options = new HashMap<>(); options.put("responsiveSnapshotCapture", true); - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with sync") public void iTakeSnapshotWithSync(String name) { Map options = new HashMap<>(); options.put("sync", true); - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with regions") public void iTakeSnapshotWithRegions(String name) { Map options = new HashMap<>(); + List> regions = regions(); if (!regions.isEmpty()) { options.put("regions", new ArrayList<>(regions)); regions.clear(); } - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with widths {string} and regions") public void iTakeSnapshotWithWidthsAndRegions(String name, String widths) { Map options = new HashMap<>(); options.put("widths", parseWidths(widths)); + List> regions = regions(); if (!regions.isEmpty()) { options.put("regions", new ArrayList<>(regions)); regions.clear(); } - percy.snapshot(name, options); + percy().snapshot(name, options); } @When("I take a Percy snapshot named {string} with options:") public void iTakeSnapshotWithOptions(String name, Map optionsTable) { Map options = buildOptions(optionsTable); + List> regions = regions(); if (!regions.isEmpty()) { options.put("regions", new ArrayList<>(regions)); regions.clear(); } - percy.snapshot(name, options); + percy().snapshot(name, options); } // ------------------------------------------------------------------ @@ -370,27 +414,29 @@ public void iTakeSnapshotWithOptions(String name, Map optionsTab @When("I take a Percy screenshot named {string}") public void iTakeScreenshot(String name) { - percy.screenshot(name); + percy().screenshot(name); } @When("I take a Percy screenshot named {string} with regions") public void iTakeScreenshotWithRegions(String name) { Map options = new HashMap<>(); + List> regions = regions(); if (!regions.isEmpty()) { options.put("regions", new ArrayList<>(regions)); regions.clear(); } - percy.screenshot(name, options); + percy().screenshot(name, options); } @When("I take a Percy screenshot named {string} with options:") public void iTakeScreenshotWithOptions(String name, Map optionsTable) { Map options = buildOptions(optionsTable); + List> regions = regions(); if (!regions.isEmpty()) { options.put("regions", new ArrayList<>(regions)); regions.clear(); } - percy.screenshot(name, options); + percy().screenshot(name, options); } // ------------------------------------------------------------------ @@ -399,7 +445,7 @@ public void iTakeScreenshotWithOptions(String name, Map optionsT @Then("Percy should be enabled") public void percyShouldBeEnabled() { - if (percy == null) { + if (percy() == null) { throw new IllegalStateException("Percy instance not initialized."); } // Percy constructor calls healthcheck; if it fails, snapshots silently no-op. @@ -410,6 +456,10 @@ public void percyShouldBeEnabled() { // Helpers // ------------------------------------------------------------------ + private static List> regions() { + return threadRegions.get(); + } + private static List parseWidths(String widths) { List result = new ArrayList<>(); for (String w : widths.split(",")) { diff --git a/src/test/java/io/percy/selenium/PercyDriverPathTest.java b/src/test/java/io/percy/selenium/PercyDriverPathTest.java index 814d41a..3bf1b9d 100644 --- a/src/test/java/io/percy/selenium/PercyDriverPathTest.java +++ b/src/test/java/io/percy/selenium/PercyDriverPathTest.java @@ -267,7 +267,7 @@ public void waitForReadySetsAndRestoresScriptTimeout() throws Exception { Map diagnostics = new HashMap(); diagnostics.put("ok", true); - when(driver.executeAsyncScript(anyString())).thenReturn(diagnostics); + when(driver.executeAsyncScript(anyString(), any())).thenReturn(diagnostics); Object result = percy.waitForReady((JavascriptExecutor) driver, new HashMap()); assertEquals(diagnostics, result); @@ -289,7 +289,7 @@ public void waitForReadyContinuesWhenTimeoutLookupThrows() throws Exception { new JSONObject().put("readiness", new JSONObject().put("timeoutMs", 1000)))); when(driver.manage()).thenThrow(new WebDriverException("no timeouts API")); - when(driver.executeAsyncScript(anyString())).thenReturn(null); + when(driver.executeAsyncScript(anyString(), any())).thenReturn(null); Object result = percy.waitForReady((JavascriptExecutor) driver, new HashMap()); assertNull(result); @@ -761,6 +761,124 @@ private static Object invokePrivate(Object target, String methodName, Class[] return method.invoke(target, args); } + // ------------------------------------------------------------------ + // Cookie forwarding: forwarded by default, opt-out via + // PERCY_DO_NOT_USE_CAPTURED_COOKIES with a one-time warning. + // ------------------------------------------------------------------ + + private Percy cookieTestPercy(RemoteWebDriver driver, Set cookies) throws Exception { + Percy percy = spy(new Percy(driver)); + setField(percy, "isPercyEnabled", true); + setField(percy, "domJs", "window.PercyDOM = window.PercyDOM || {};"); + setField(percy, "cliConfig", new JSONObject().put("snapshot", new JSONObject())); + percy.sessionType = "web"; + + WebDriver.Options options = mock(WebDriver.Options.class); + when(driver.manage()).thenReturn(options); + when(options.getCookies()).thenReturn(cookies); + when(driver.getCurrentUrl()).thenReturn("https://example.com"); + when(driver.findElements(By.tagName("iframe"))).thenReturn(Collections.emptyList()); + when(((JavascriptExecutor) driver).executeScript(anyString())).thenReturn(new HashMap()); + return percy; + } + + @Test + public void snapshotForwardsSessionCookiesByDefault() throws Exception { + RemoteWebDriver driver = mock(RemoteWebDriver.class); + Set cookies = new HashSet<>(Arrays.asList( + new Cookie("session_id", "abc123"), new Cookie("auth_token", "tok"))); + Percy percy = cookieTestPercy(driver, cookies); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(JSONObject.class); + doReturn(new JSONObject()).when(percy).request(eq("/percy/snapshot"), bodyCaptor.capture(), eq("cookies_default")); + + percy.snapshot("cookies_default"); + + // Round-trip through the wire format the CLI receives. + org.json.JSONArray sent = new JSONObject(bodyCaptor.getValue().toString()) + .getJSONObject("domSnapshot").getJSONArray("cookies"); + assertEquals(2, sent.length()); + Map byName = new HashMap<>(); + for (int i = 0; i < sent.length(); i++) { + byName.put(sent.getJSONObject(i).getString("name"), sent.getJSONObject(i).getString("value")); + } + assertEquals("abc123", byName.get("session_id")); + assertEquals("tok", byName.get("auth_token")); + } + + @Test + public void snapshotSendsNoCookiesAndWarnsOnceWhenOptedOut() throws Exception { + java.io.PrintStream originalOut = System.out; + java.io.ByteArrayOutputStream buf = new java.io.ByteArrayOutputStream(); + setStaticField(Percy.class, "PERCY_DO_NOT_USE_CAPTURED_COOKIES", true); + Field warned = Percy.class.getDeclaredField("cookieOptOutWarned"); + warned.setAccessible(true); + ((java.util.concurrent.atomic.AtomicBoolean) warned.get(null)).set(false); + try { + System.setOut(new java.io.PrintStream(buf, true)); + RemoteWebDriver driver = mock(RemoteWebDriver.class); + Set cookies = new HashSet<>(Collections.singletonList(new Cookie("session_id", "abc123"))); + Percy percy = cookieTestPercy(driver, cookies); + + ArgumentCaptor bodyCaptor = ArgumentCaptor.forClass(JSONObject.class); + doReturn(new JSONObject()).when(percy).request(eq("/percy/snapshot"), bodyCaptor.capture(), anyString()); + + percy.snapshot("opt_out_1"); + percy.snapshot("opt_out_2"); + + for (JSONObject body : bodyCaptor.getAllValues()) { + assertEquals(0, new JSONObject(body.toString()).getJSONObject("domSnapshot").getJSONArray("cookies").length()); + } + assertEquals(2, bodyCaptor.getAllValues().size()); + verify(driver.manage(), never()).getCookies(); + + // Cookies handed straight to getSerializedDOM are dropped too. + Map direct = percy.getSerializedDOM( + (JavascriptExecutor) driver, cookies, new HashMap()); + assertTrue(((Set) direct.get("cookies")).isEmpty()); + } finally { + System.setOut(originalOut); + setStaticField(Percy.class, "PERCY_DO_NOT_USE_CAPTURED_COOKIES", false); + } + String out = buf.toString(); + String warning = "Not sending browser cookies to Percy because PERCY_DO_NOT_USE_CAPTURED_COOKIES is set. " + + "Pages or assets that need a logged-in session may fail to load during asset discovery, " + + "so snapshots can render incorrectly."; + assertEquals(1, out.split(java.util.regex.Pattern.quote(warning), -1).length - 1, out); + } + + // ------------------------------------------------------------------ + // waitForReady: readiness config travels as a script argument. + // ------------------------------------------------------------------ + + @Test + @SuppressWarnings("unchecked") + public void waitForReadyPassesConfigAsScriptArgument() throws Exception { + RemoteWebDriver driver = mock(RemoteWebDriver.class); + Percy percy = spy(new Percy(driver)); + String payload = "custom\"}; window.__pwned = 1; var _={\"x"; + setField(percy, "cliConfig", new JSONObject().put("snapshot", + new JSONObject().put("readiness", new JSONObject().put("preset", payload)))); + + Map perSnapshot = new HashMap<>(); + perSnapshot.put("stabilityWindowMs", 100); + Map options = new HashMap<>(); + options.put("readiness", perSnapshot); + + percy.waitForReady((JavascriptExecutor) driver, options); + + ArgumentCaptor scriptCap = ArgumentCaptor.forClass(String.class); + ArgumentCaptor argCap = ArgumentCaptor.forClass(Object.class); + verify(driver).executeAsyncScript(scriptCap.capture(), argCap.capture()); + String script = scriptCap.getValue(); + assertTrue(script.startsWith("var cfg = arguments[0];"), script); + assertFalse(script.contains("__pwned"), script); + assertFalse(script.contains("stabilityWindowMs"), script); + Map cfg = (Map) argCap.getValue(); + assertEquals(payload, cfg.get("preset")); + assertEquals(100, cfg.get("stabilityWindowMs")); + } + private static void setField(Object target, String fieldName, Object value) throws Exception { Field field = Percy.class.getDeclaredField(fieldName); field.setAccessible(true); diff --git a/src/test/java/io/percy/selenium/PercyLogicTest.java b/src/test/java/io/percy/selenium/PercyLogicTest.java index 9bc1db9..d56ead3 100644 --- a/src/test/java/io/percy/selenium/PercyLogicTest.java +++ b/src/test/java/io/percy/selenium/PercyLogicTest.java @@ -486,6 +486,26 @@ public void getSerializedDomAttachesCookiesAndSkipsWhenNoIframes() throws Except @Test public void getSerializedDomCapturesCrossOriginIframe() throws Exception { + Map frame = serializeWithCrossOriginIframe(); + // Default: the frame's own cookies are kept as PercyDOM returned them. + assertEquals("frame_session=1", ((Map) frame.get("iframeSnapshot")).get("cookies")); + } + + @Test + public void getSerializedDomStripsIframeCookiesWhenOptedOut() throws Exception { + setStaticField(Percy.class, "PERCY_DO_NOT_USE_CAPTURED_COOKIES", true); + try { + Map frame = serializeWithCrossOriginIframe(); + Map iframeSnapshot = (Map) frame.get("iframeSnapshot"); + assertFalse(iframeSnapshot.containsKey("cookies")); + assertEquals("iframe", iframeSnapshot.get("dom")); + } finally { + setStaticField(Percy.class, "PERCY_DO_NOT_USE_CAPTURED_COOKIES", false); + } + } + + @SuppressWarnings("unchecked") + private Map serializeWithCrossOriginIframe() throws Exception { RemoteWebDriver mockedDriver = mock(RemoteWebDriver.class); Percy mockedPercy = spy(new Percy(mockedDriver)); setField(mockedPercy, "domJs", "window.PercyDOM = window.PercyDOM || {};"); @@ -506,6 +526,7 @@ public void getSerializedDomCapturesCrossOriginIframe() throws Exception { mainSnapshot.put("dom", "main"); Map iframeSnapshot = new HashMap(); iframeSnapshot.put("dom", "iframe"); + iframeSnapshot.put("cookies", "frame_session=1"); when(((JavascriptExecutor) mockedDriver).executeScript(anyString())).thenAnswer(invocation -> { String script = invocation.getArgument(0); @@ -525,6 +546,7 @@ public void getSerializedDomCapturesCrossOriginIframe() throws Exception { List> corsIframes = (List>) serialized.get("corsIframes"); assertEquals(1, corsIframes.size()); assertEquals("https://cdn.other.com/frame", corsIframes.get(0).get("frameUrl")); + return corsIframes.get(0); } @Test @@ -535,7 +557,7 @@ public void getSerializedDomAttachesReadinessDiagnostics() throws Exception { Map diagnostics = new HashMap(); diagnostics.put("ok", true); - when(((JavascriptExecutor) mockedDriver).executeAsyncScript(anyString())).thenReturn(diagnostics); + when(((JavascriptExecutor) mockedDriver).executeAsyncScript(anyString(), any())).thenReturn(diagnostics); Map domSnap = new HashMap(); domSnap.put("html", ""); when(((JavascriptExecutor) mockedDriver).executeScript(anyString())).thenReturn(domSnap); @@ -563,7 +585,7 @@ public void getSerializedDomSkipsReadinessWhenPresetDisabled() throws Exception Map result = mockedPercy.getSerializedDOM( (JavascriptExecutor) mockedDriver, new HashSet(), options); - verify((JavascriptExecutor) mockedDriver, never()).executeAsyncScript(anyString()); + verify((JavascriptExecutor) mockedDriver, never()).executeAsyncScript(anyString(), any()); assertNull(result.get("readiness_diagnostics")); } @@ -602,7 +624,7 @@ public void waitForReadyReturnsNullWhenPresetDisabled() throws Exception { Object result = mockedPercy.waitForReady((JavascriptExecutor) mockedDriver, options); assertNull(result); - verify((JavascriptExecutor) mockedDriver, never()).executeAsyncScript(anyString()); + verify((JavascriptExecutor) mockedDriver, never()).executeAsyncScript(anyString(), any()); } @Test @@ -611,7 +633,7 @@ public void waitForReadyReturnsNullWhenAsyncScriptThrows() throws Exception { Percy mockedPercy = spy(new Percy(mockedDriver)); setField(mockedPercy, "cliConfig", new JSONObject().put("snapshot", new JSONObject())); - when(((JavascriptExecutor) mockedDriver).executeAsyncScript(anyString())) + when(((JavascriptExecutor) mockedDriver).executeAsyncScript(anyString(), any())) .thenThrow(new RuntimeException("boom")); Object result = mockedPercy.waitForReady((JavascriptExecutor) mockedDriver, new HashMap()); diff --git a/src/test/java/io/percy/selenium/SdkTest.java b/src/test/java/io/percy/selenium/SdkTest.java index eeda302..95f6508 100644 --- a/src/test/java/io/percy/selenium/SdkTest.java +++ b/src/test/java/io/percy/selenium/SdkTest.java @@ -1122,7 +1122,7 @@ public void readinessRunsBeforeSerializeAndAttachesDiagnostics() throws Exceptio diagnostics.put("ok", true); diagnostics.put("timed_out", false); // executeAsyncScript (readiness) - when(((JavascriptExecutor) mockedDriver).executeAsyncScript(any(String.class))).thenReturn(diagnostics); + when(((JavascriptExecutor) mockedDriver).executeAsyncScript(any(String.class), any())).thenReturn(diagnostics); // executeScript (serialize + any other sync scripts) Map domSnap = new HashMap<>(); domSnap.put("html", ""); @@ -1133,7 +1133,7 @@ public void readinessRunsBeforeSerializeAndAttachesDiagnostics() throws Exceptio // Readiness script was sent via executeAsyncScript ArgumentCaptor scriptCap = ArgumentCaptor.forClass(String.class); - verify((JavascriptExecutor) mockedDriver, atLeastOnce()).executeAsyncScript(scriptCap.capture()); + verify((JavascriptExecutor) mockedDriver, atLeastOnce()).executeAsyncScript(scriptCap.capture(), any()); assertTrue(scriptCap.getValue().contains("waitForReady"), "readiness script should mention waitForReady"); // Diagnostics propagated to the snapshot @@ -1159,7 +1159,7 @@ public void readinessSkippedWhenPresetDisabled() throws Exception { (JavascriptExecutor) mockedDriver, new HashSet<>(), options); // executeAsyncScript must NOT have been called - verify((JavascriptExecutor) mockedDriver, never()).executeAsyncScript(any(String.class)); + verify((JavascriptExecutor) mockedDriver, never()).executeAsyncScript(any(String.class), any()); // serialize still ran; no diagnostics attached assertNull(result.get("readiness_diagnostics")); } @@ -1171,7 +1171,7 @@ public void snapshotSurvivesReadinessThrow() throws Exception { setField(mockedPercy, "isPercyEnabled", true); setField(mockedPercy, "cliConfig", new JSONObject().put("snapshot", new JSONObject())); - when(((JavascriptExecutor) mockedDriver).executeAsyncScript(any(String.class))) + when(((JavascriptExecutor) mockedDriver).executeAsyncScript(any(String.class), any())) .thenThrow(new RuntimeException("readiness boom")); Map domSnap = new HashMap<>(); domSnap.put("html", ""); diff --git a/src/test/java/io/percy/selenium/cucumber/PercyStepsTest.java b/src/test/java/io/percy/selenium/cucumber/PercyStepsTest.java index 29d60c0..493adca 100644 --- a/src/test/java/io/percy/selenium/cucumber/PercyStepsTest.java +++ b/src/test/java/io/percy/selenium/cucumber/PercyStepsTest.java @@ -11,7 +11,6 @@ import org.mockito.ArgumentCaptor; import org.openqa.selenium.WebDriver; -import java.lang.reflect.Field; import java.util.*; class PercyStepsTest { @@ -38,13 +37,7 @@ private void initWithMockPercy() { } private void setPercyField(Percy percy) { - try { - Field field = PercySteps.class.getDeclaredField("percy"); - field.setAccessible(true); - field.set(null, percy); - } catch (Exception e) { - throw new RuntimeException(e); - } + PercySteps.setPercy(percy); } // ------------------------------------------------------------------ @@ -546,6 +539,72 @@ void testGetCucumberVersionReturnsValueWhenManifestPresent() throws Exception { assertEquals("9.9.9", version); } + // ------------------------------------------------------------------ + // Parallel scenarios: per-thread driver/percy/regions + // ------------------------------------------------------------------ + + @Test + void testSetDriverOnAnotherThreadDoesNotReplaceThisThreadsPercy() throws Exception { + PercySteps.setDriver(mockDriver); + Percy mine = PercySteps.getPercy(); + + Percy[] theirs = new Percy[1]; + WebDriver otherDriver = mock(WebDriver.class); + Thread other = new Thread(() -> { + PercySteps.setDriver(otherDriver); + theirs[0] = PercySteps.getPercy(); + }); + other.start(); + other.join(30000); + + assertNotNull(theirs[0]); + assertNotSame(mine, theirs[0]); + assertSame(mine, PercySteps.getPercy()); + } + + @Test + void testThreadWithoutOwnDriverFallsBackToLastSetDriver() throws Exception { + PercySteps.setDriver(mockDriver); + Percy shared = PercySteps.getPercy(); + + Percy[] seen = new Percy[1]; + Thread other = new Thread(() -> { + seen[0] = PercySteps.getPercy(); + new PercySteps().iHaveAPercyInstance(); + }); + other.start(); + other.join(30000); + + assertSame(shared, seen[0]); + } + + @Test + void testRegionsDoNotLeakAcrossThreads() throws Exception { + Percy otherPercy = mock(Percy.class); + Map region = new HashMap<>(); + region.put("algorithm", "ignore"); + when(otherPercy.createRegion(anyMap())).thenReturn(region); + + // Another scenario thread creates a region but never snapshots. + WebDriver otherDriver = mock(WebDriver.class); + Thread other = new Thread(() -> { + PercySteps.setDriver(otherDriver); + PercySteps.setPercy(otherPercy); + new PercySteps().iCreateIgnoreRegionCSS(".ad"); + }); + other.start(); + other.join(30000); + verify(otherPercy).createRegion(anyMap()); + + initWithMockPercy(); + steps.iTakeSnapshotWithRegions("Mine"); + + @SuppressWarnings("unchecked") + ArgumentCaptor> captor = ArgumentCaptor.forClass(Map.class); + verify(mockPercy).snapshot(eq("Mine"), captor.capture()); + assertFalse(captor.getValue().containsKey("regions")); + } + @Test void testGetCucumberVersionFallsBackToUnknownWhenManifestMissing() throws Exception { // resolver returns null (no manifest) -> "unknown".