Skip to content

Commit 0af90e5

Browse files
committed
Fix: Correct translation of leading slash in Linux/OS X paths to/from a URL.
This fixes rewrites of URL in Markdown to long relative paths.
1 parent 1c0df6d commit 0af90e5

File tree

4 files changed

+53
-18
lines changed

4 files changed

+53
-18
lines changed

server/src/webserver.rs

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -520,8 +520,16 @@ pub async fn filesystem_endpoint(
520520
// On Windows, backslashes in the `request_file_path` will be treated as
521521
// path separators; however, HTTP does not treat them as path separators.
522522
// Therefore, re-encode them to prevent inconsistency between the way HTTP
523-
// and this program interpret file paths.
523+
// and this program interpret file paths. On OS X/Linux, the path starts
524+
// with a leading slash, which gets absorbed into the URL to prevent a URL
525+
// such as "/fw/fsc/1//foo/bar/...". Restore it here.
526+
#[cfg(target_os = "windows")]
524527
let fixed_file_path = request_file_path.replace("\\", "%5C");
528+
// On OS X/Linux, the path starts with a leading slash, which gets absorbed
529+
// into the URL to prevent a URL such as "/fw/fsc/1//foo/bar/...". Restore
530+
// it here.
531+
#[cfg(not(target_os = "windows"))]
532+
let fixed_file_path = format!("/{request_file_path}");
525533
let file_path = match try_canonicalize(&fixed_file_path) {
526534
Ok(v) => v,
527535
Err(err) => {
@@ -658,9 +666,9 @@ async fn text_file_to_response(
658666
) -> (
659667
// The response to send back to the HTTP endpoint.
660668
SimpleHttpResponse,
661-
// If the response is a Client, also return the appropriate `Update`
662-
// data to populate the Client with the parsed `file_contents`. In all other
663-
// cases, return None.
669+
// If the response is a Client, also return the appropriate `Update` data to
670+
// populate the Client with the parsed `file_contents`. In all other cases,
671+
// return None.
664672
Option<EditorMessageContents>,
665673
) {
666674
// Use a lossy conversion, since this is UI display, not filesystem access.
@@ -1322,7 +1330,22 @@ fn path_to_url(prefix: &str, connection_id: &str, file_path: &Path) -> String {
13221330
// Then put it all back together.
13231331
.collect::<Vec<_>>()
13241332
.join("/");
1325-
format!("{prefix}/{connection_id}/{pathname}")
1333+
// On Windows, path names start with a drive letter. On Linux/OS X, they
1334+
// start with a forward slash -- don't put a double forward slash in the
1335+
// resulting path.
1336+
format!("{prefix}/{connection_id}/{}", drop_leading_slash(&pathname))
1337+
}
1338+
1339+
// Given a string (which is probably a pathname), drop the leading slash if it's
1340+
// present.
1341+
fn drop_leading_slash(path_: &str) -> &str {
1342+
if path_.starts_with("/") {
1343+
let mut chars = path_.chars();
1344+
chars.next();
1345+
chars.as_str()
1346+
} else {
1347+
path_
1348+
}
13261349
}
13271350

13281351
// Given a `Path`, transform it into a displayable HTML string (with any

server/src/webserver/filewatcher.rs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -744,7 +744,7 @@ mod tests {
744744
source_to_codechat_for_web,
745745
},
746746
test_utils::{check_logger_errors, configure_testing_logger},
747-
webserver::{IdeType, ResultOkTypes, tests::IP_PORT},
747+
webserver::{IdeType, ResultOkTypes, drop_leading_slash, tests::IP_PORT},
748748
};
749749

750750
async fn get_websocket_queues(
@@ -822,15 +822,21 @@ mod tests {
822822
.unwrap()
823823
.map(|s| urlencoding::decode(s).unwrap())
824824
.collect();
825-
let url_path = PathBuf::from_str(&url_segs[3..].join("/"))
826-
.unwrap()
827-
.canonicalize()
828-
.unwrap();
825+
let mut url_path = if cfg!(windows) {
826+
PathBuf::new()
827+
} else {
828+
PathBuf::from_str("/").unwrap()
829+
};
830+
url_path.push(PathBuf::from_str(&url_segs[3..].join("/")).unwrap());
831+
let url_path = url_path.canonicalize().unwrap();
829832
assert_eq!(url_path, test_path);
830833
send_response(&ide_tx_queue, id, Ok(ResultOkTypes::Void)).await;
831834

832835
// 2. After fetching the file, we should get an update.
833-
let uri = format!("/fw/fsc/1/{}/test.py", test_dir.to_slash().unwrap());
836+
let uri = format!(
837+
"/fw/fsc/1/{}/test.py",
838+
drop_leading_slash(&test_dir.to_slash().unwrap())
839+
);
834840
let req = test::TestRequest::get().uri(&uri).to_request();
835841
let resp = test::call_service(&app, req).await;
836842
assert!(resp.status().is_success());
@@ -869,7 +875,10 @@ mod tests {
869875
.to_str()
870876
.unwrap()
871877
.to_string();
872-
let uri = format!("/fw/fsc/1/{}/test.py", test_dir.to_slash().unwrap());
878+
let uri = format!(
879+
"/fw/fsc/1/{}/test.py",
880+
drop_leading_slash(&test_dir.to_slash().unwrap())
881+
);
873882
let req = test::TestRequest::get().uri(&uri).to_request();
874883
let resp = test::call_service(&app, req).await;
875884
assert!(resp.status().is_success());
@@ -1050,7 +1059,7 @@ mod tests {
10501059
new_file_path.push("test1.py");
10511060
let new_uri = format!(
10521061
"http://localhost/fw/fsc/1/{}",
1053-
urlencoding::encode(&new_file_path.to_slash().unwrap())
1062+
drop_leading_slash(&urlencoding::encode(&new_file_path.to_slash().unwrap()))
10541063
);
10551064
ide_tx_queue
10561065
.send(EditorMessage {

server/src/webserver/tests.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use std::{
2424
};
2525

2626
use assert_cmd::Command;
27-
use assertables::{assert_ends_with, assert_starts_with};
27+
use assertables::{assert_ends_with, assert_not_contains, assert_starts_with};
2828

2929
use super::{filewatcher::FILEWATCHER_PATH_PREFIX, path_to_url, url_to_path};
3030
use crate::prep_test_dir;
@@ -106,6 +106,8 @@ fn test_path_to_url() {
106106
let url = path_to_url("/a/b", "conn1", &file_path);
107107
assert_starts_with!(url, "/a/b/conn1/");
108108
assert_ends_with!(url, "test_path_to_url/test%20spaces.py");
109+
// There shouldn't be a double forward slash in the name.
110+
assert_not_contains!(url, "//");
109111
// Report any errors produced when removing the temporary directory.
110112
temp_dir.close().unwrap();
111113
}

server/src/webserver/vscode.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,8 @@ pub async fn vscode_ide_websocket(
414414
file_path: clean_file_path.to_str().expect("Since the path started as a string, assume it losslessly translates back to a string.").to_string(),
415415
contents: Some(CodeChatForWeb {
416416
metadata: SourceFileMetadata {
417-
// Since this is raw data, `mode` doesn't matter.
417+
// Since this is raw data, `mode` doesn't
418+
// matter.
418419
mode: "".to_string()
419420
},
420421
source: CodeMirror {
@@ -739,7 +740,7 @@ mod test {
739740
cast,
740741
processing::{CodeChatForWeb, CodeMirror, SourceFileMetadata},
741742
test_utils::{_prep_test_dir, check_logger_errors, configure_testing_logger},
742-
webserver::{ResultOkTypes, UpdateMessageContents},
743+
webserver::{ResultOkTypes, UpdateMessageContents, drop_leading_slash},
743744
};
744745

745746
lazy_static! {
@@ -972,7 +973,7 @@ mod test {
972973
open_client(&mut ws_ide).await;
973974

974975
let file_path = test_dir.join("none.py");
975-
let file_path_str = file_path.to_slash().unwrap().to_string();
976+
let file_path_str = drop_leading_slash(&file_path.to_slash().unwrap()).to_string();
976977

977978
// Do this is a thread, since the request generates a message that
978979
// requires a response in order to complete.
@@ -1128,7 +1129,7 @@ mod test {
11281129
assert_eq!(
11291130
minreq::get(format!(
11301131
"http://localhost:8080/vsc/fs/{connection_id}/{}",
1131-
file_path_thread.to_slash().unwrap()
1132+
drop_leading_slash(&file_path_thread.to_slash().unwrap())
11321133
))
11331134
.send()
11341135
.unwrap()

0 commit comments

Comments
 (0)