fix(js): do not redefine img src when a page made it non-configurable (#418)

Booking.com's anti-bot script pre-defines a non-configurable own `src` on
<img> elements. The Image shim's load emulation then threw "TypeError:
Cannot redefine property: src" out of the constructor, crashing any page
that calls new Image() after that instrumentation runs.

Check the own descriptor first and skip the load emulation when `src` is
already non-configurable: a page that owns `src` is instrumenting loads
itself. The normal path (configurable src, emulated load event) is
unchanged and now covered by a test.

Fixes the crash half of #394.
This commit is contained in:
Jim Zhang @ wonderhoi authored and GitHub committed 2026-07-16 12:41:28 +02:00
1 parent 113b492ed5
commit 891d850d19
2 files changed
+124 -15

No files matched your search

+22 -15
View File
@@ -7398,21 +7398,28 @@ if (typeof Image === 'undefined') {
// `.src` flips `complete` and fires `load` on a microtask-later tick. Lazy // `.src` flips `complete` and fires `load` on a microtask-later tick. Lazy
// loaders and preloaders that create `new Image()`, set `.src`, and wait for // loaders and preloaders that create `new Image()`, set `.src`, and wait for
// `onload` (or addEventListener('load')) would hang forever otherwise. // `onload` (or addEventListener('load')) would hang forever otherwise.
Object.defineProperty(img, 'src', { // Anti-bot scripts (Booking.com, issue #394) pre-define a non-configurable
configurable: true, enumerable: true, // own `src` on <img> elements; redefining it throws "Cannot redefine
get() { return _imgSrcDesc.get.call(img); }, // property: src" and kills the constructor. Skip the load emulation then:
set(v) { // a page that owns `src` is instrumenting loads itself.
_imgSrcDesc.set.call(img, v); const ownSrc = Object.getOwnPropertyDescriptor(img, 'src');
if (!img.getAttribute('src')) return; if (!ownSrc || ownSrc.configurable) {
img.complete = false; Object.defineProperty(img, 'src', {
setTimeout(function () { configurable: true, enumerable: true,
img.complete = true; get() { return _imgSrcDesc.get.call(img); },
img.naturalWidth = img.naturalWidth || img.width || 0; set(v) {
img.naturalHeight = img.naturalHeight || img.height || 0; _imgSrcDesc.set.call(img, v);
try { img.dispatchEvent(new Event('load')); } catch (e) {} if (!img.getAttribute('src')) return;
}, 0); img.complete = false;
}, setTimeout(function () {
}); img.complete = true;
img.naturalWidth = img.naturalWidth || img.width || 0;
img.naturalHeight = img.naturalHeight || img.height || 0;
try { img.dispatchEvent(new Event('load')); } catch (e) {}
}, 0);
},
});
}
return img; return img;
}; };
globalThis.Image.prototype = globalThis.HTMLImageElement.prototype; globalThis.Image.prototype = globalThis.HTMLImageElement.prototype;
+102
View File
@@ -0,0 +1,102 @@
//! Regression test for issue #394 (crash half): `new Image()` must survive a
//! page that pre-defines a non-configurable own `src` on `<img>` elements, the
//! way Booking.com's anti-bot instrumentation does. The Image shim used to
//! unconditionally redefine `src` on the element it just created, throwing
//! `TypeError: Cannot redefine property: src`.
use std::io::{Read, Write};
use obscura::Browser;
/// Minimal HTTP/1.1 server returning HTML that wraps document.createElement to
/// plant a non-configurable `src` on every <img>, then calls `new Image()`.
fn spawn_server(html: &'static str) -> String {
let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
let addr = listener.local_addr().unwrap();
std::thread::spawn(move || {
for incoming in listener.incoming() {
let mut s = match incoming {
Ok(s) => s,
Err(_) => continue,
};
let mut buf = [0u8; 2048];
let _ = s.read(&mut buf);
let resp = format!(
"HTTP/1.1 200 OK\r\nContent-Type: text/html\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}",
html.len(),
html
);
let _ = s.write_all(resp.as_bytes());
let _ = s.shutdown(std::net::Shutdown::Both);
}
});
format!("http://{}", addr)
}
#[tokio::test]
async fn new_image_survives_non_configurable_src() {
std::env::set_var("OBSCURA_ALLOW_PRIVATE_NETWORK", "1");
let base = spawn_server(
r#"<!doctype html><html><body><div id="r">waiting</div>
<script>
var origCreate = document.createElement.bind(document);
document.createElement = function (tag) {
var el = origCreate(tag);
if (String(tag).toLowerCase() === 'img') {
Object.defineProperty(el, 'src', { value: '', writable: true, configurable: false });
}
return el;
};
var img = new Image(10, 20);
document.getElementById('r').textContent =
'survived w=' + img.width + ' h=' + img.height;
</script>
</body></html>"#,
);
let browser = Browser::new().unwrap();
let mut page = browser.new_page().await.unwrap();
page.goto(&base).await.unwrap();
let text = page.evaluate("document.getElementById('r').textContent");
assert_eq!(
text.as_str().unwrap_or(""),
"survived w=10 h=20",
"new Image() threw instead of degrading when src is non-configurable"
);
}
#[tokio::test]
async fn new_image_still_emulates_load_when_src_is_configurable() {
std::env::set_var("OBSCURA_ALLOW_PRIVATE_NETWORK", "1");
let base = spawn_server(
r#"<!doctype html><html><body><div id="r">waiting</div>
<script>
var img = new Image();
img.onload = function () {
document.getElementById('r').textContent = 'loaded complete=' + img.complete;
};
img.src = '/pixel.png';
</script>
</body></html>"#,
);
let browser = Browser::new().unwrap();
let mut page = browser.new_page().await.unwrap();
page.goto(&base).await.unwrap();
// The shim fires `load` on a setTimeout(0); pump the event loop.
for _ in 0..10 {
page.settle(500).await;
let text = page.evaluate("document.getElementById('r').textContent");
if text.as_str().unwrap_or("").starts_with("loaded") {
break;
}
}
let text = page.evaluate("document.getElementById('r').textContent");
assert_eq!(
text.as_str().unwrap_or(""),
"loaded complete=true",
"load emulation regressed for the normal (configurable src) path"
);
}