Compare commits

..
Author SHA1 Message Date
Brandon Thomas 04c5613b2c Merge remote-tracking branch 'origin/main' into fix2708
# Conflicts:
#	crates/io/src/tests.rs
2026-11-03 09:07:28 -05:00
Brandon ThomasandClaude Opus 5.5 4c6a9bcb56 Test: assert the empty table is read (CodeRabbit)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-03 08:57:21 -05:00
Dylan M. Taylor 1881f1ed59 fix(io): an ACAD_TABLE with no rows or no columns reads as an empty table
The table cell budget charges rows x columns, which is zero when either dimension is, so a table declaring 10 000 rows and no columns still allocated row heights and empty row vectors for free, once per record. Such a table now keeps neither dimension.
2026-11-03 11:10:06 +00:00
6 changed files with 10 additions and 122 deletions
Generated
-1
View File
@@ -772,7 +772,6 @@ dependencies = [
"cadcraft-render",
"clap",
"getrandom 0.4.3",
"libc",
"log",
"serde_json",
]
+1 -11
View File
@@ -333,17 +333,7 @@ pub(super) fn apply_sortents(
paper_for: &HashMap<String, String>,
first_layout: &str,
) {
// Each sort rebuilds its whole space, so a space named by many tables is sorted once, by the
// last of them (which decided its order anyway); repeated tables made import quadratic.
let target = |owner: &String| {
let up = brs.get(owner)?.to_ascii_uppercase();
Some(if up.starts_with("*PAPER_SPACE") { paper_for.get(&up).cloned().unwrap_or_else(|| first_layout.to_string()) + "\0paper" } else { up })
};
let last: HashMap<String, usize> = tables.iter().enumerate().filter_map(|(i, (owner, _))| Some((target(owner)?, i))).collect();
for (i, (owner, map)) in tables.iter().enumerate() {
if target(owner).and_then(|t| last.get(&t)) != Some(&i) {
continue;
}
for (owner, map) in tables {
let Some(name) = brs.get(owner) else { continue };
let up = name.to_ascii_uppercase();
// A sort handle of 0 draws last (ezdxf `reorder`, which notes AutoCAD defines it, undocumented).
-22
View File
@@ -337,28 +337,6 @@ fn entity_transparency_roundtrips_as_group_440() {
assert_eq!(d.model.iter().map(|e| e.common.transparency).collect::<Vec<_>>(), [Transparency::ByLayer, Transparency::Percent(90)]);
}
/// Several SORTENTSTABLEs for one space sort it once, in the last table's order.
#[test]
fn repeated_sortents_tables_use_the_last() {
let table =
|a: u32, b: u32| format!("0\nSORTENTSTABLE\n5\n{:X}\n100\nAcDbSortentsTable\n330\n1F\n331\nA\n5\n{a:X}\n331\nB\n5\n{b:X}\n", 0x100 + a);
let mut t = String::from(
"0\nSECTION\n2\nTABLES\n0\nTABLE\n2\nBLOCK_RECORD\n0\nBLOCK_RECORD\n5\n1F\n2\n*Model_Space\n0\nENDTAB\n0\nENDSEC\n\
0\nSECTION\n2\nENTITIES\n0\nLINE\n5\nA\n330\n1F\n10\n0\n20\n0\n11\n1\n21\n1\n0\nLINE\n5\nB\n330\n1F\n10\n0\n20\n0\n11\n2\n21\n2\n0\nENDSEC\n\
0\nSECTION\n2\nOBJECTS\n0\nDICTIONARY\n5\nC\n",
);
for _ in 0..1000 {
t.push_str(&table(2, 1));
}
t.push_str(&table(1, 2));
t.push_str("0\nENDSEC\n0\nEOF\n");
let d = read(t.as_bytes(), "s.dxf").unwrap();
assert_eq!(d.model.iter().map(|e| e.handle.0).collect::<Vec<_>>(), [0xA, 0xB]);
let reversed = t.replacen(&table(1, 2), &table(2, 1), 1);
let d = read(reversed.as_bytes(), "s.dxf").unwrap();
assert_eq!(d.model.iter().map(|e| e.handle.0).collect::<Vec<_>>(), [0xB, 0xA]);
}
/// A table declaring 10 000 rows and no columns (or the reverse) is read as an empty table.
#[test]
fn table_with_an_empty_dimension_allocates_nothing() {
-4
View File
@@ -17,7 +17,3 @@ serde_json = { workspace = true }
log = { workspace = true }
# Secure randomness for the per-session control token (MIT OR Apache-2.0).
getrandom = "0.4"
[target.'cfg(unix)'.dependencies]
# O_NOFOLLOW when reading the token file (MIT OR Apache-2.0).
libc = "0.2"
+7 -38
View File
@@ -1,7 +1,7 @@
//! Where MCP tool calls end up: a control-channel method call.
use std::io::{BufRead, BufReader, Read, Write};
use std::net::{Ipv4Addr, SocketAddr, TcpStream, ToSocketAddrs};
use std::net::{TcpStream, ToSocketAddrs};
use std::time::Duration;
use cadcraft_engine::Session;
@@ -76,17 +76,9 @@ impl Remote {
/// The per-session token for this port, read (and cached) from the owner-only token file. A
/// missing file is not fatal here: the request is sent without a token and the server replies
/// with an authentication error the user can act on.
///
/// The desktop app listens only on `127.0.0.1:PORT`, so the token goes only to that exact
/// peer: a name that also resolves elsewhere (`localhost` → `::1`, where another local account
/// may listen on the same port) must not be handed the desktop's credential (CWE-522).
fn token(&mut self) -> Option<&str> {
let port = port_of(&self.addr)?;
let peer = self.conn.as_ref().and_then(|(_, s)| s.peer_addr().ok());
if peer != Some(SocketAddr::from((Ipv4Addr::LOCALHOST, port))) {
return None;
}
if self.token.is_none()
&& let Some(port) = port_of(&self.addr)
&& let Ok(t) = crate::control_token::read(port)
{
self.token = Some(t);
@@ -215,14 +207,6 @@ fn is_read_only(method: &str) -> bool {
impl Remote {
/// Put the current token (if any) on `req`, replacing the one it carries.
/// Connect if needed, put the token for the peer actually connected to on `req`, and send it.
fn sign_and_send(&mut self, req: &mut Value) -> Result<String, (std::io::Error, bool)> {
if self.conn.is_none() {
self.reach().map_err(|e| (e, false))?;
}
self.set_token(req);
self.roundtrip(&req.to_string())
}
fn set_token(&mut self, req: &mut Value) {
let token = self.token().map(str::to_string);
if let Some(obj) = req.as_object_mut() {
@@ -235,7 +219,7 @@ impl Remote {
/// Send `req` and parse the reply, refreshing the token before a safe retry after a transport
/// error. A mutation that may have reached the server is never resent automatically.
fn exchange(&mut self, method: &str, req: &mut Value) -> Result<Value, String> {
let reply = match self.sign_and_send(req) {
let reply = match self.roundtrip(&req.to_string()) {
Ok(r) => r,
Err((e, sent)) => {
self.token = None;
@@ -248,7 +232,8 @@ impl Remote {
));
}
// A restarted server may have rotated its token before dropping the old socket.
self.sign_and_send(req).map_err(|(e, _)| {
self.set_token(req);
self.roundtrip(&req.to_string()).map_err(|(e, _)| {
self.token = None;
self.conn = None;
format!("CADCraft at {}: {e}", self.addr)
@@ -271,12 +256,14 @@ impl Backend for Remote {
let id = self.next_id;
self.next_id += 1;
let mut req = json!({"id": id, "method": method, "params": params});
self.set_token(&mut req);
let mut v = self.exchange(method, &mut req)?;
// The token is read before `--launch` opens the app, which then writes a new one (#2087); the
// file can also be left over from an earlier session. Once, so a wrong token can't loop.
if unauthorized(&v) && v.get("state").and_then(Value::as_str) == Some("not-run") {
self.token = None;
self.conn = None;
self.set_token(&mut req);
v = self.exchange(method, &mut req)?;
}
if v.get("ok").and_then(Value::as_bool) == Some(true) {
@@ -602,24 +589,6 @@ mod tests {
})
}
/// A listener on another loopback address with the desktop's port (here `::1`) never receives
/// the desktop's token.
#[test]
fn token_is_not_sent_to_another_loopback_listener() {
let Ok(listener) = TcpListener::bind("[::1]:0") else { return };
let addr = listener.local_addr().unwrap();
crate::control_token::issue(addr.port()).unwrap();
let server = fake_app(listener, move |req, _| {
assert!(req.get("token").is_none(), "token leaked to {addr}");
json!({"ok": true, "result": "x"})
});
let mut remote = Remote::lazy(&addr.to_string(), false);
let got = remote.call("ping", json!({}));
crate::control_token::revoke(addr.port());
assert_eq!(got, Ok(json!("x")));
assert_eq!(server.join().unwrap(), 1);
}
/// `--launch` reads the token before it opens the app, so the first request carries the token
/// of an earlier session (or none); the app refuses it without running it, and the call is
/// resent once with the token the app has just written (#2087).
+2 -46
View File
@@ -32,9 +32,7 @@ fn private_dir() -> io::Result<PathBuf> {
if let Some(h) = std::env::var_os("HOME").filter(|v| !v.is_empty()) {
return Ok(PathBuf::from(h).join(".cache"));
}
// The shared temporary directory would put the credential at a path any local account can
// predict and plant a link at: refuse instead.
Err(io::Error::new(io::ErrorKind::NotFound, "XDG_RUNTIME_DIR or HOME is required for a private control token"))
Ok(std::env::temp_dir())
}
/// Require the Windows per-user local app-data directory for session token storage.
@@ -77,31 +75,7 @@ fn issue_at(path: &std::path::Path, bytes: &[u8]) -> io::Result<()> {
/// Read the token for `port`, trimming trailing whitespace. Used by clients (the MCP bridge).
pub fn read(port: u16) -> io::Result<String> {
read_at(&token_path(port)?)
}
/// Read a token file the issuer wrote: not through a link, a plain file, and (Unix) readable by
/// its owner only, so a file another account planted or left open is not taken as the credential.
fn read_at(path: &std::path::Path) -> io::Result<String> {
use std::io::Read;
let mut options = std::fs::OpenOptions::new();
options.read(true);
#[cfg(unix)]
{
use std::os::unix::fs::OpenOptionsExt;
options.custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK);
}
let file = options.open(path)?;
let meta = file.metadata()?;
if !meta.is_file() {
return Err(io::Error::new(io::ErrorKind::InvalidData, "the control token is not a plain file"));
}
#[cfg(unix)]
if std::os::unix::fs::PermissionsExt::mode(&meta.permissions()) & 0o077 != 0 {
return Err(io::Error::new(io::ErrorKind::PermissionDenied, "the control token file is readable by other users"));
}
let mut raw = String::new();
file.take(1024).read_to_string(&mut raw)?;
let raw = std::fs::read_to_string(token_path(port)?)?;
let token = raw.trim().to_string();
if token.is_empty() {
return Err(io::Error::new(io::ErrorKind::InvalidData, "empty control token"));
@@ -207,24 +181,6 @@ mod tests {
assert_eq!(windows_private_dir(Some("private-app-data".into())).unwrap(), PathBuf::from("private-app-data"));
}
/// A link, or a file other users can read, is not accepted as the token.
#[cfg(unix)]
#[test]
fn read_refuses_links_and_shared_files() {
use std::os::unix::fs::PermissionsExt;
let dir = std::env::temp_dir().join(format!("cadcraft-token-read-{}", std::process::id()));
std::fs::create_dir_all(&dir).unwrap();
let secret = dir.join("secret");
issue_at(&secret, b"abc").unwrap();
assert_eq!(read_at(&secret).unwrap(), "abc");
let link = dir.join("link");
std::os::unix::fs::symlink(&secret, &link).unwrap();
assert!(read_at(&link).is_err());
std::fs::set_permissions(&secret, std::fs::Permissions::from_mode(0o644)).unwrap();
assert!(read_at(&secret).is_err());
std::fs::remove_dir_all(dir).unwrap();
}
/// Token issuance creates its missing directory hierarchy before writing the credential.
#[test]
fn issuance_creates_missing_parent() {