From a1faedf53f5267e9607077816acbbdb7fe17097f Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Sat, 7 Oct 2023 16:40:41 +0900 Subject: [PATCH 1/4] Remove SeekExt --- src/hg_connect_http.rs | 4 ++-- src/hg_connect_stdio.rs | 4 ++-- src/util.rs | 13 +------------ 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/hg_connect_http.rs b/src/hg_connect_http.rs index c0339d20f..5e834ba7f 100644 --- a/src/hg_connect_http.rs +++ b/src/hg_connect_http.rs @@ -45,7 +45,7 @@ use crate::libgit::{ credential_fill, curl_errorstr, get_active_slot, http_auth, http_follow_config, run_one_slot, slot_results, ssl_cainfo, HTTP_OK, HTTP_REAUTH, }; -use crate::util::{ImmutBString, OsStrExt, PrefixWriter, ReadExt, SeekExt, SliceExt, ToBoxed}; +use crate::util::{ImmutBString, OsStrExt, PrefixWriter, ReadExt, SliceExt, ToBoxed}; use self::git_http_state::{GitHttpStateToken, GIT_HTTP_STATE}; @@ -230,7 +230,7 @@ impl HttpRequest { curl_easy_setopt( slot.curl, CURLOPT_POSTFIELDSIZE_LARGE, - body.stream_len_().unwrap(), + body.seek(SeekFrom::End(0)).unwrap(), ); /* Ensure we have no state from a previous attempt that failed because * of authentication (401). */ diff --git a/src/hg_connect_stdio.rs b/src/hg_connect_stdio.rs index dc8f6cc05..71b6e8b29 100644 --- a/src/hg_connect_stdio.rs +++ b/src/hg_connect_stdio.rs @@ -25,7 +25,7 @@ use crate::hg_connect::{ use crate::libc::FdFile; use crate::libcinnabar::{hg_connect_stdio, stdio_finish}; use crate::libgit::child_process; -use crate::util::{ImmutBString, OsStrExt, PrefixWriter, ReadExt, SeekExt}; +use crate::util::{ImmutBString, OsStrExt, PrefixWriter, ReadExt}; pub struct HgStdioConnection { capabilities: HgCapabilities, @@ -115,7 +115,7 @@ impl HgWireConnection for HgStdioConnection { self.proc_in.write_all(&header).unwrap(); drop(header); - let len = input.stream_len_().unwrap(); + let len = input.metadata().unwrap().len(); //TODO: chunk in smaller pieces. writeln!(self.proc_in, "{}", len).unwrap(); diff --git a/src/util.rs b/src/util.rs index 690952fcd..4c1f4bdb2 100644 --- a/src/util.rs +++ b/src/util.rs @@ -4,7 +4,7 @@ use std::ffi::{CStr, CString, OsStr}; use std::fmt; -use std::io::{self, LineWriter, Read, Seek, SeekFrom, Write}; +use std::io::{self, LineWriter, Read, Write}; use std::mem; #[cfg(unix)] use std::os::unix::ffi; @@ -95,17 +95,6 @@ pub trait ReadExt: Read { impl ReadExt for T {} -pub trait SeekExt: Seek { - fn stream_len_(&mut self) -> io::Result { - let old_pos = self.stream_position()?; - let len = self.seek(SeekFrom::End(0))?; - self.seek(SeekFrom::Start(old_pos))?; - Ok(len) - } -} - -impl SeekExt for T {} - pub trait SliceExt { fn splitn_exact(&self, c: C) -> Option<[&Self; N]>; fn rsplitn_exact(&self, c: C) -> Option<[&Self; N]>; From 127c77711c4c6f3caefddd68e05604342011d208 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Sat, 7 Oct 2023 17:30:04 +0900 Subject: [PATCH 2/4] Wrap HTTP request body in a utility struct We're going to use the struct to handle httppostargs. --- src/hg_connect_http.rs | 154 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 141 insertions(+), 13 deletions(-) diff --git a/src/hg_connect_http.rs b/src/hg_connect_http.rs index 5e834ba7f..9ce08ca3a 100644 --- a/src/hg_connect_http.rs +++ b/src/hg_connect_http.rs @@ -4,6 +4,7 @@ use std::borrow::ToOwned; use std::cmp; +use std::convert::AsRef; use std::ffi::{c_void, CStr, CString, OsStr}; use std::fs::File; use std::io::{self, copy, stderr, Cursor, Read, Seek, SeekFrom, Write}; @@ -130,14 +131,141 @@ pub struct HgHttpConnection { * The command results are simply the corresponding HTTP responses. */ -trait ReadAndSeek: Read + Seek {} +/// A `Read` that knows its exact length and can seek to its beginning. +trait ExactSizeReadRewind: Read { + fn len(&self) -> io::Result; -impl ReadAndSeek for T {} + fn rewind(&mut self) -> io::Result<()>; +} + +impl ExactSizeReadRewind for File { + fn len(&self) -> io::Result { + self.metadata().map(|m| m.len()) + } + + fn rewind(&mut self) -> io::Result<()> { + self.seek(SeekFrom::Start(0)).map(|_| ()) + } +} + +impl> ExactSizeReadRewind for Cursor { + fn len(&self) -> io::Result { + Ok(self.get_ref().as_ref().len().try_into().unwrap()) + } + + fn rewind(&mut self) -> io::Result<()> { + self.set_position(0); + Ok(()) + } +} + +enum Body { + None, + Simple(Box), + Chained( + std::io::Chain, Box>, + ), +} + +impl Body { + fn new() -> Body { + Body::None + } + + fn is_some(&self) -> bool { + !matches!(self, Body::None) + } + + fn add(&mut self, r: impl ExactSizeReadRewind + Send + 'static) { + let current = mem::replace(self, Body::None); + *self = match current { + Body::None => Body::Simple(Box::new(r)), + Body::Simple(first) => Body::Chained(first.chain(Box::new(r))), + Body::Chained(_) => Body::Chained( + (Box::new(current) as Box).chain(Box::new(r)), + ), + } + } +} + +impl Read for Body { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + match self { + Body::None => Ok(0), + Body::Simple(first) => first.read(buf), + Body::Chained(chained) => chained.read(buf), + } + } +} + +impl ExactSizeReadRewind for Body { + fn len(&self) -> io::Result { + match self { + Body::None => Ok(0), + Body::Simple(first) => first.len(), + Body::Chained(chained) => { + let (first, second) = chained.get_ref(); + Ok(first.len()? + second.len()?) + } + } + } + + fn rewind(&mut self) -> io::Result<()> { + match self { + Body::None => Ok(()), + Body::Simple(first) => first.rewind(), + Body::Chained(_) => { + let current = mem::replace(self, Body::None); + if let Body::Chained(chained) = current { + let (mut first, mut second) = chained.into_inner(); + first.rewind()?; + second.rewind()?; + *self = Body::Chained(first.chain(second)); + } + Ok(()) + } + } + } +} + +#[test] +fn test_exactsize_read_rewind_body() { + let a = "abcd"; + let b = "efg"; + let c = "hijklm"; + + let mut body1 = Body::new(); + body1.add(Cursor::new(&a[..])); + + assert_eq!(body1.len().unwrap(), 4); + assert_eq!(&body1.read_all_to_string().unwrap()[..], "abcd"); + body1.rewind().unwrap(); + assert_eq!(&body1.read_all_to_string().unwrap()[..], "abcd"); + + let mut body2 = Body::new(); + body2.add(Cursor::new(&a[..])); + body2.add(Cursor::new(&b[..])); + + assert_eq!(body2.len().unwrap(), 7); + assert_eq!(&body2.read_all_to_string().unwrap()[..], "abcdefg"); + body2.rewind().unwrap(); + assert_eq!(&body2.read_all_to_string().unwrap()[..], "abcdefg"); + + let mut body3 = Body::new(); + body3.add(Cursor::new(&a[..])); + body3.add(Cursor::new(&b[..])); + body3.add(Cursor::new(&c[..])); + + assert_eq!(body3.len().unwrap(), 13); + assert_eq!(&body3.read_all_to_string().unwrap()[..], "abcdefghijklm"); + body3.rewind().unwrap(); + assert_eq!(&body3.read_all_to_string().unwrap()[..], "abcdefghijklm"); +} pub struct HttpRequest { url: Url, headers: Vec<(String, String)>, - body: Option>, + body: Body, follow_redirects: bool, token: Arc, } @@ -175,7 +303,7 @@ impl HttpRequest { HttpRequest { url, headers: Vec::new(), - body: None, + body: Body::new(), follow_redirects: false, token: Arc::new(token), } @@ -189,8 +317,8 @@ impl HttpRequest { self.headers.push((name.to_string(), value.to_string())); } - fn post_data(&mut self, data: Box) { - self.body = Some(data); + fn post_data(&mut self, data: impl ExactSizeReadRewind + Send + 'static) { + self.body.add(data); } #[allow(clippy::result_large_err)] @@ -225,21 +353,21 @@ impl HttpRequest { http_request_execute as *const c_void, ); let mut headers = ptr::null_mut(); - if let Some(ref mut body) = self.body { + if self.body.is_some() { curl_easy_setopt(slot.curl, CURLOPT_POST, 1); curl_easy_setopt( slot.curl, CURLOPT_POSTFIELDSIZE_LARGE, - body.seek(SeekFrom::End(0)).unwrap(), + self.body.len().unwrap(), ); /* Ensure we have no state from a previous attempt that failed because * of authentication (401). */ - body.seek(SeekFrom::Start(0)).unwrap(); - curl_easy_setopt(slot.curl, CURLOPT_READDATA, &mut *body); + self.body.rewind().unwrap(); + curl_easy_setopt(slot.curl, CURLOPT_READDATA, &mut self.body); curl_easy_setopt( slot.curl, CURLOPT_READFUNCTION, - read_from_read::<&mut (dyn ReadAndSeek + Send)> as *const c_void, + read_from_read:: as *const c_void, ); curl_easy_setopt(slot.curl, CURLOPT_FOLLOWLOCATION, 0); headers = curl_slist_append(headers, cstr!("Expect:").as_ptr()); @@ -535,7 +663,7 @@ impl HgWireConnection for HgHttpConnection { let mut http_req = self.start_command_request(command, args); if command == "pushkey" { http_req.header("Content-Type", "application/mercurial-0.1"); - http_req.post_data(Box::new(Cursor::new(Vec::::new()))); + http_req.post_data(Cursor::new(Vec::::new())); } let mut http_resp = http_req.execute().unwrap(); self.handle_redirect(&http_resp); @@ -588,7 +716,7 @@ impl HgWireConnection for HgHttpConnection { fn push_command(&mut self, input: File, command: &str, args: HgArgs) -> UnbundleResponse { let mut http_req = self.start_command_request(command, args); - http_req.post_data(Box::new(input)); + http_req.post_data(input); http_req.header("Content-Type", "application/mercurial-0.1"); let mut http_resp = http_req.execute().unwrap(); self.handle_redirect(&http_resp); From fc848ba4c65a8b309e2d1d74432dba068e99ed29 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Sat, 7 Oct 2023 18:02:28 +0900 Subject: [PATCH 3/4] Add support for httppostargs --- src/hg_connect_http.rs | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/hg_connect_http.rs b/src/hg_connect_http.rs index 9ce08ca3a..1f9a21d32 100644 --- a/src/hg_connect_http.rs +++ b/src/hg_connect_http.rs @@ -606,28 +606,36 @@ impl HgHttpConnection { .and_then(|s| usize::from_str(s).ok()) .unwrap_or(0); + let httppostargs = self.get_capability(b"httppostargs").is_some(); + let mut command_url = self.url.clone(); let mut query_pairs = command_url.query_pairs_mut(); query_pairs.append_pair("cmd", command); let mut headers = Vec::new(); + let mut body = None; - if httpheader > 0 && !args.is_empty() { + if !args.is_empty() && (httppostargs || httpheader > 0) { let mut encoder = form_urlencoded::Serializer::new(String::new()); for (name, value) in args.iter() { encoder.append_pair(name, value); } let args = encoder.finish(); - let mut args = &args[..]; - let mut num = 1; - while !args.is_empty() { - let header_name = format!("X-HgArg-{}", num); - num += 1; - let (chunk, remainder) = args.split_at(cmp::min( - args.len(), - httpheader - header_name.len() - ": ".len(), - )); - headers.push((header_name, chunk.to_string())); - args = remainder; + if httppostargs { + headers.push(("X-HgArgs-Post".to_string(), args.len().to_string())); + body = Some(args); + } else { + let mut args = &args[..]; + let mut num = 1; + while !args.is_empty() { + let header_name = format!("X-HgArg-{}", num); + num += 1; + let (chunk, remainder) = args.split_at(cmp::min( + args.len(), + httpheader - header_name.len() - ": ".len(), + )); + headers.push((header_name, chunk.to_string())); + args = remainder; + } } } else { for (name, value) in args.iter() { @@ -645,6 +653,9 @@ impl HgHttpConnection { for (name, value) in headers { request.header(&name, &value); } + if let Some(body) = body { + request.post_data(Cursor::new(body)); + } request } From 230cce330ea7a566e6ae3c00ae8832917733dec4 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Sun, 8 Oct 2023 07:03:49 +0900 Subject: [PATCH 4/4] [CI] Enable httppostargs in mercurial during tests --- CI/tests.mk | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CI/tests.mk b/CI/tests.mk index f617ff66d..aa8517e0e 100644 --- a/CI/tests.mk +++ b/CI/tests.mk @@ -157,7 +157,7 @@ hg.clonebundles-full.git: hg.clonebundles-full.hg hg.git hg.clonebundles-bz2.git: hg.clonebundles-bz2.hg hg.git hg.clonebundles-full-bz2.git: hg.clonebundles-full-bz2.hg hg.git hg.clonebundles.git hg.clonebundles-full.git hg.clonebundles-bz2.git hg.clonebundles-full-bz2.git: - $(HG) -R $< --config serve.other=http --config serve.otherport=88$(NUM) --config web.port=80$(NUM) --config extensions.clonebundles= --config extensions.x=$(TOPDIR)/CI/hg-serve-exec.py serve-and-exec -- $(GIT) clone --progress -n hg://localhost:80$(NUM).http/ $@ + $(HG) -R $< --config serve.other=http --config serve.otherport=88$(NUM) --config web.port=80$(NUM) --config experimental.httppostargs=true --config extensions.clonebundles= --config extensions.x=$(TOPDIR)/CI/hg-serve-exec.py serve-and-exec -- $(GIT) clone --progress -n hg://localhost:80$(NUM).http/ $@ $(call COMPARE_REFS, $(word 2,$^), $@) hg.pure.git: hg.git @@ -179,7 +179,7 @@ hg.http.hg.gitcredentials hg.http.hg.nobundle2.gitcredentials: hg.http.hg hg.http.hg.nobundle2: %: %.gitcredentials hg.git $(call HG_INIT, $@) - $(HG) -R $@ --config extensions.x=$(TOPDIR)/CI/hg-serve-exec.py --config web.port=80$(NUM) serve-and-exec -- $(GIT) -c credential.helper='store --file=$(CURDIR)/$@.gitcredentials' -c cinnabar.data=never -C $(word 2,$^) push hg://localhost:80$(NUM).http/ refs/remotes/origin/*:refs/heads/* + $(HG) -R $@ --config experimental.httppostargs=true --config extensions.x=$(TOPDIR)/CI/hg-serve-exec.py --config web.port=80$(NUM) serve-and-exec -- $(GIT) -c credential.helper='store --file=$(CURDIR)/$@.gitcredentials' -c cinnabar.data=never -C $(word 2,$^) push hg://localhost:80$(NUM).http/ refs/remotes/origin/*:refs/heads/* hg.incr.base.git: hg.incr.hg $(HG) clone -U $< $@.hg @@ -213,7 +213,7 @@ hg.cinnabarclone-bundle.git hg.cinnabarclone-bundle-full.git hg.cinnabarclone-gr hg.cinnabarclone.git hg.cinnabarclone-full.git hg.cinnabarclone-bundle.git hg.cinnabarclone-bundle-full.git hg.cinnabarclone-graft.git hg.cinnabarclone-graft-replace.git: hg.pure.hg $(HG) clone -U $< $@.hg echo http://localhost:88$(NUM)/$(word 2,$^) | tee $@.hg/.hg/cinnabar.manifest - $(HG) -R $@.hg --config web.port=80$(NUM) --config serve.other=$(OTHER_SERVER) --config serve.otherport=88$(NUM) --config extensions.x=$(TOPDIR)/CI/hg-serve-exec.py --config extensions.cinnabarclone=$(HG_CINNABARCLONE_EXT) serve-and-exec -- $(GIT) clone --progress hg://localhost:80$(NUM).http/ $@ + $(HG) -R $@.hg --config web.port=80$(NUM) --config serve.other=$(OTHER_SERVER) --config serve.otherport=88$(NUM) --config experimental.httppostargs=true --config extensions.x=$(TOPDIR)/CI/hg-serve-exec.py --config extensions.cinnabarclone=$(HG_CINNABARCLONE_EXT) serve-and-exec -- $(GIT) clone --progress hg://localhost:80$(NUM).http/ $@ $(call COMPARE_REFS, $(or $(word 3,$^),$(word 2,$^)), $@) $(GIT) -C $@ cinnabar fsck $(GIT) -C $@ cinnabar fsck --full @@ -224,7 +224,7 @@ hg.cinnabarclone-graft-bundle.git: hg.pure.hg $(GIT) -C $@ cinnabar rollback 0000000000000000000000000000000000000000 $(GIT) -C $@ remote rename origin grafted (echo http://localhost:88$(NUM)/$(word 2,$^); echo http://localhost:88$(NUM)/$(word 4,$^) graft=$$($(GIT) ls-remote $(CURDIR)/$(word 4,$^) refs/cinnabar/replace/* | awk -F/ '{print $$NF}')) | tee $@.hg/.hg/cinnabar.manifest - $(HG) -R $@.hg --config serve.other=http --config serve.otherport=88$(NUM) --config web.port=80$(NUM) --config extensions.x=$(TOPDIR)/CI/hg-serve-exec.py --config extensions.cinnabarclone=$(HG_CINNABARCLONE_EXT) serve-and-exec -- $(GIT) -c cinnabar.graft=true -C $@ fetch --progress hg://localhost:80$(NUM).http/ refs/heads/*:refs/remotes/origin/* + $(HG) -R $@.hg --config serve.other=http --config serve.otherport=88$(NUM) --config web.port=80$(NUM) --config experimental.httppostargs=true --config extensions.x=$(TOPDIR)/CI/hg-serve-exec.py --config extensions.cinnabarclone=$(HG_CINNABARCLONE_EXT) serve-and-exec -- $(GIT) -c cinnabar.graft=true -C $@ fetch --progress hg://localhost:80$(NUM).http/ refs/heads/*:refs/remotes/origin/* $(call COMPARE_REFS, $(or $(word 3,$^),$(word 2,$^)), $@) $(GIT) -C $@ cinnabar fsck $(GIT) -C $@ cinnabar fsck --full