Per request output takes file-root into account for path resolving

This commit is contained in:
jcamiel 2024-02-28 18:05:50 +01:00 committed by hurl-bot
parent 2641b66ece
commit 2433a24887
No known key found for this signature in database
GPG Key ID: 1283A2B4A0DCAF8D
18 changed files with 134 additions and 62 deletions

View File

@ -0,0 +1,3 @@
curl --output build/fileroot.bin 'http://localhost:8000/fileroot'
curl --header 'Content-Type:' --data-binary '@build/fileroot.bin' 'http://localhost:8000/fileroot'
curl --header 'Content-Type:' --data-binary '@build/../build/fileroot.bin' 'http://localhost:8000/fileroot'

View File

@ -0,0 +1,19 @@
# --file-root access is currently used for input and output files that are referenced in a Hurl file.
# User explicitly sets a "file root" through the command line, which acts as the parent of relatives path used
# in the Hurl file. Without specifying this "file root", the Hurl file's directory is used as "file root".
GET http://localhost:8000/fileroot
[Options]
output: fileroot.bin
HTTP 200
POST http://localhost:8000/fileroot
file,fileroot.bin;
HTTP 200
# Paths are OK if they are descendant of the file root.
POST http://localhost:8000/fileroot
file,../build/fileroot.bin;
HTTP 200

View File

View File

@ -0,0 +1,6 @@
Set-StrictMode -Version latest
$ErrorActionPreference = 'Stop'
if (Test-Path build/fileroot.bin) {
Remove-Item build/fileroot.bin
}
hurl --file-root build/ tests_ok/fileroot.hurl

View File

@ -0,0 +1,13 @@
from app import app
from flask import request
@app.route("/fileroot", methods=["GET", "POST"])
def fileroot():
data = '{"user":"bob","age":21}'
if request.method == "POST":
s = request.data.decode("utf-8")
assert s == data
return ""
else:
return data

View File

@ -0,0 +1,4 @@
#!/bin/bash
set -Eeuo pipefail
rm -f build/fileroot.bin
hurl --file-root build/ tests_ok/fileroot.hurl

View File

@ -1,2 +1,2 @@
curl --header 'Content-Type: application/json' --data '{ "user": "bob" }' --output build/output_request_1.bin 'http://localhost:8000/output/endpoint1'
curl --header 'Content-Type: application/json' --data '{ "user": "bob" }' --output build/../build/output_request_1.bin 'http://localhost:8000/output/endpoint1'
curl 'http://localhost:8000/output/endpoint2'

View File

@ -1,6 +1,6 @@
POST http://localhost:8000/output/endpoint1
[Options]
output: build/output_request_1.bin
output: ../build/output_request_1.bin
{ "user": "bob" }
HTTP 200

View File

@ -3,5 +3,5 @@ $ErrorActionPreference = 'Stop'
if (Test-Path build/output_request_1.bin) {
Remove-Item build/output_request_1.bin
}
hurl --no-output tests_ok/output_option.hurl
hurl --no-output --file-root build tests_ok/output_option.hurl
Write-Host (Get-Content build/output_request_1.bin -Raw) -NoNewLine

View File

@ -1,5 +1,5 @@
#!/bin/bash
set -Eeuo pipefail
rm -f build/output_request_1.bin
hurl --no-output tests_ok/output_option.hurl
hurl --no-output --file-root build tests_ok/output_option.hurl
cat build/output_request_1.bin

View File

@ -364,8 +364,8 @@ impl Options {
let compressed = self.compressed;
let connect_timeout = self.connect_timeout;
let connects_to = self.connects_to.clone();
let file_root = match self.file_root {
Some(ref filename) => Path::new(filename),
let file_root = match &self.file_root {
Some(filename) => Path::new(filename),
None => {
if filename == "-" {
current_dir

View File

@ -743,9 +743,17 @@ impl Client {
arguments.append(&mut options.curl_args());
// --output is not an option of the HTTP client, we deal with it here:
if let Some(output) = output {
arguments.push("--output".to_string());
arguments.push(output.to_string());
match output {
Some(Output::File(filename)) => {
let filename = context_dir.get_path(filename);
arguments.push("--output".to_string());
arguments.push(filename.into_os_string().into_string().unwrap());
}
Some(Output::StdOut) => {
arguments.push("--output".to_string());
arguments.push("-".to_string());
}
None => {}
}
arguments.push(url);

View File

@ -34,8 +34,8 @@ pub fn write_json(
let s = format!("{serialized}\n");
let bytes = s.into_bytes();
match filename_out {
Some(Output::File(file)) => Output::File(file.to_string()).write(&bytes)?,
_ => Output::StdOut.write(&bytes)?,
Some(Output::File(file)) => Output::File(file.to_string()).write(&bytes, None)?,
_ => Output::StdOut.write(&bytes, None)?,
}
Ok(())
}

View File

@ -65,8 +65,8 @@ pub fn write_body(
output.extend(bytes);
}
match filename_out {
Some(Output::File(file)) => Output::File(file.to_string()).write(&output)?,
_ => runner::Output::StdOut.write(&output)?,
Some(Output::File(file)) => Output::File(file.to_string()).write(&output, None)?,
_ => runner::Output::StdOut.write(&output, None)?,
}
} else {
logger.info("No response has been received");

View File

@ -16,7 +16,6 @@
*
*/
use std::collections::HashMap;
use std::path::PathBuf;
use std::thread;
use std::time::Instant;
@ -28,7 +27,7 @@ use hurl_core::parser;
use crate::http::Call;
use crate::runner::runner_options::RunnerOptions;
use crate::runner::{entry, options, EntryResult, HurlResult, Output, RunnerError, Value};
use crate::runner::{entry, options, EntryResult, HurlResult, Value};
use crate::util::logger::{ErrorFormat, Logger, LoggerOptions, LoggerOptionsBuilder};
use crate::{http, runner};
@ -201,7 +200,7 @@ pub fn run(
log_errors(&entry_result, content, retry, &logger);
}
// When --output is overriden on a request level, we output the HTTP response only if the
// When --output is overridden on a request level, we output the HTTP response only if the
// call has succeeded.
if let Ok(RunnerOptions {
output: Some(output),
@ -214,24 +213,11 @@ pub fn run(
// an error. If we want to treat it as an error, we've to add it to the current
// `entry_result` errors, and optionally deals with retry if we can't write to the
// specified path.
let authorized = if let Output::File(filename) = &output {
if !runner_options.context_dir.is_access_allowed(filename) {
let inner = RunnerError::UnauthorizedFileAccess {
path: PathBuf::from(filename.clone()),
};
let error = runner::Error::new(entry.request.source_info, inner, false);
logger.warning(&error.fixme());
false
} else {
true
}
} else {
true
};
if authorized {
if let Err(error) = entry_result.write_response(&output) {
logger.warning(&error.fixme());
}
let source_info = entry.source_info();
if let Err(error) =
entry_result.write_response(&output, &runner_options.context_dir, source_info)
{
logger.warning(&error.fixme());
}
}
}

View File

@ -22,6 +22,7 @@ use std::io::Write;
use std::{fmt, io};
use crate::runner::{Error, RunnerError};
use crate::util::path::ContextDir;
use hurl_core::ast::{Pos, SourceInfo};
/// Represents the output of write operation: can be either a file or stdout.
@ -43,20 +44,35 @@ impl fmt::Display for Output {
impl Output {
/// Writes these `bytes` to the output.
pub fn write(&self, bytes: &[u8]) -> Result<(), Error> {
pub fn write(&self, bytes: &[u8], context_dir: Option<&ContextDir>) -> Result<(), Error> {
match self {
Output::StdOut => match write_stdout(bytes) {
Ok(_) => Ok(()),
Err(e) => Err(Error::new_file_write_access("stdout", &e.to_string())),
},
Output::File(filename) => {
let mut file = match File::create(filename) {
// If we have a context dir, we check if we can write to this filename and compute
// the new filename given this context dir.
let filename = match context_dir {
None => filename.to_string(),
Some(context_dir) => {
if !context_dir.is_access_allowed(filename) {
return Err(Error::new_file_write_access(
filename,
"unauthorized context dir",
));
}
let path = context_dir.get_path(filename);
path.into_os_string().into_string().unwrap()
}
};
let mut file = match File::create(&filename) {
Ok(file) => file,
Err(e) => return Err(Error::new_file_write_access(filename, &e.to_string())),
Err(e) => return Err(Error::new_file_write_access(&filename, &e.to_string())),
};
match file.write_all(bytes) {
Ok(_) => Ok(()),
Err(e) => Err(Error::new_file_write_access(filename, &e.to_string())),
Err(e) => Err(Error::new_file_write_access(&filename, &e.to_string())),
}
}
}

View File

@ -16,11 +16,14 @@
*
*/
use hurl_core::ast::{Pos, SourceInfo};
use std::path::PathBuf;
use crate::http::{Call, Cookie};
use crate::runner::error::Error;
use crate::runner::output::Output;
use crate::runner::value::Value;
use crate::runner::RunnerError;
use crate::util::path::ContextDir;
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct HurlResult {
@ -127,30 +130,43 @@ pub struct CaptureResult {
pub type PredicateResult = Result<(), Error>;
impl EntryResult {
/// Writes the last HTTP response of this entry result to the file `filename`.
/// Writes the last HTTP response of this entry result to this `output`.
/// The HTTP response can be decompressed if the entry's `compressed` option has been set.
pub fn write_response(&self, output: &Output) -> Result<(), Error> {
match self.calls.last() {
Some(call) => {
let response = &call.response;
if self.compressed {
let bytes = match response.uncompress_body() {
Ok(bytes) => bytes,
Err(e) => {
// TODO: pass a [`SourceInfo`] in case of error
// We may pass a [`SourceInfo`] as a parameter of this method to make
// a more accurate error (for instance a [`SourceInfo`] pointing at
// `output: foo.bin`
let source_info = SourceInfo::new(Pos::new(0, 0), Pos::new(0, 0));
return Err(Error::new(source_info, e.into(), false));
}
};
output.write(&bytes)
} else {
output.write(&response.body)
}
/// This method checks if the response has write access to this output, given a `context_dir`.
pub fn write_response(
&self,
output: &Output,
context_dir: &ContextDir,
source_info: SourceInfo,
) -> Result<(), Error> {
let Some(call) = self.calls.last() else {
return Ok(());
};
// We check file access authorization for file output when a context dir has been given
if let Output::File(filename) = output {
if !context_dir.is_access_allowed(filename) {
let inner = RunnerError::UnauthorizedFileAccess {
path: PathBuf::from(filename.clone()),
};
return Err(Error::new(source_info, inner, false));
}
None => Ok(()),
}
let response = &call.response;
if self.compressed {
let bytes = match response.uncompress_body() {
Ok(bytes) => bytes,
Err(e) => {
// TODO: pass a [`SourceInfo`] in case of error
// We may pass a [`SourceInfo`] as a parameter of this method to make
// a more accurate error (for instance a [`SourceInfo`] pointing at
// `output: foo.bin`
let source_info = SourceInfo::new(Pos::new(0, 0), Pos::new(0, 0));
return Err(Error::new(source_info, e.into(), false));
}
};
output.write(&bytes, Some(context_dir))
} else {
output.write(&response.body, Some(context_dir))
}
}
}

View File

@ -53,8 +53,9 @@ impl ContextDir {
self.file_root.join(Path::new(filename))
}
/// Checks if a given filename access is authorized.
/// This method is used to check if a local file can be included in POST request.
/// Checks if a given `filename` access is authorized.
/// This method is used to check if a local file can be included in POST request or if a
/// response can be outputted to a given file when using `output` option in [Options] sections.
pub fn is_access_allowed(&self, filename: &str) -> bool {
let file = self.get_path(filename);
let absolute_file = self.current_dir.join(file);