From d9d6cea5a9204d8faa938b8344b35c8d47e6dffd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Habov=C5=A1tiak?= Date: Mon, 21 Nov 2022 02:06:09 +0100 Subject: [PATCH] Make json require string and pass around metadata (#7010) * Make json require string and pass around metadata The json deserializer was accepting any inputs by coercing non-strings into strings. As an example, if the input was `[1, 2]` the coercion would turn into `[12]` and deserialize as a list containing number twelve instead of a list of two numbers, one and two. This could lead to silent data corruption. Aside from that pipeline metadata wasn't passed aroud. This commit fixes the type issue by adding a strict conversion function that errors if the input type is not a string or external stream. It then uses this function instead of the original `collect_string()`. In addition, this function returns the pipeline metadata so it can be passed along. * Make other formats require string The problem with json coercing non-string types to string was present in all other text formats. This reuses the `collect_string_strict` function to fix them. * `IntoPipelineData` cleanup The method `into_pipeline_data_with_metadata` can now be conveniently used. --- crates/nu-command/src/formats/from/csv.rs | 3 +- .../nu-command/src/formats/from/delimited.rs | 7 ++- crates/nu-command/src/formats/from/eml.rs | 9 ++-- crates/nu-command/src/formats/from/ics.rs | 15 +++---- crates/nu-command/src/formats/from/ini.rs | 14 +++--- crates/nu-command/src/formats/from/json.rs | 11 ++--- crates/nu-command/src/formats/from/nuon.rs | 7 ++- crates/nu-command/src/formats/from/ssv.rs | 5 +-- crates/nu-command/src/formats/from/toml.rs | 7 ++- crates/nu-command/src/formats/from/tsv.rs | 10 +---- crates/nu-command/src/formats/from/url.rs | 15 +++---- crates/nu-command/src/formats/from/vcf.rs | 15 +++---- crates/nu-command/src/formats/from/xml.rs | 15 +++---- crates/nu-command/src/formats/from/yaml.rs | 21 +++++---- crates/nu-protocol/src/pipeline_data.rs | 45 ++++++++++++++++--- 15 files changed, 104 insertions(+), 95 deletions(-) diff --git a/crates/nu-command/src/formats/from/csv.rs b/crates/nu-command/src/formats/from/csv.rs index e71a07b6b..363a2324e 100644 --- a/crates/nu-command/src/formats/from/csv.rs +++ b/crates/nu-command/src/formats/from/csv.rs @@ -116,7 +116,6 @@ fn from_csv( let noheaders = call.has_flag("noheaders"); let separator: Option = call.get_flag(engine_state, stack, "separator")?; let trim: Option = call.get_flag(engine_state, stack, "trim")?; - let config = engine_state.get_config(); let sep = match separator { Some(Value::String { val: s, span }) => { @@ -138,7 +137,7 @@ fn from_csv( let trim = trim_from_str(trim)?; - from_delimited_data(noheaders, no_infer, sep, trim, input, name, config) + from_delimited_data(noheaders, no_infer, sep, trim, input, name) } #[cfg(test)] diff --git a/crates/nu-command/src/formats/from/delimited.rs b/crates/nu-command/src/formats/from/delimited.rs index c95159858..3cacd8fc5 100644 --- a/crates/nu-command/src/formats/from/delimited.rs +++ b/crates/nu-command/src/formats/from/delimited.rs @@ -1,5 +1,5 @@ use csv::{ReaderBuilder, Trim}; -use nu_protocol::{Config, IntoPipelineData, PipelineData, ShellError, Span, Value}; +use nu_protocol::{IntoPipelineData, PipelineData, ShellError, Span, Value}; fn from_delimited_string_to_value( s: String, @@ -63,14 +63,13 @@ pub fn from_delimited_data( trim: Trim, input: PipelineData, name: Span, - config: &Config, ) -> Result { - let concat_string = input.collect_string("", config)?; + let (concat_string, metadata) = input.collect_string_strict(name)?; Ok( from_delimited_string_to_value(concat_string, noheaders, no_infer, sep, trim, name) .map_err(|x| ShellError::DelimiterError(x.to_string(), name))? - .into_pipeline_data(), + .into_pipeline_data_with_metadata(metadata), ) } diff --git a/crates/nu-command/src/formats/from/eml.rs b/crates/nu-command/src/formats/from/eml.rs index 2da207976..4fd6bbaa9 100644 --- a/crates/nu-command/src/formats/from/eml.rs +++ b/crates/nu-command/src/formats/from/eml.rs @@ -5,7 +5,6 @@ use nu_engine::CallExt; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::Category; -use nu_protocol::Config; use nu_protocol::{ Example, PipelineData, ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value, }; @@ -46,8 +45,7 @@ impl Command for FromEml { let head = call.head; let preview_body: Option> = call.get_flag(engine_state, stack, "preview-body")?; - let config = engine_state.get_config(); - from_eml(input, preview_body, head, config) + from_eml(input, preview_body, head) } fn examples(&self) -> Vec { @@ -182,9 +180,8 @@ fn from_eml( input: PipelineData, preview_body: Option>, head: Span, - config: &Config, ) -> Result { - let value = input.collect_string("", config)?; + let (value, metadata) = input.collect_string_strict(head)?; let body_preview = preview_body .map(|b| b.item as usize) @@ -236,7 +233,7 @@ fn from_eml( item: collected, span: head, }), - None, + metadata, )) } diff --git a/crates/nu-command/src/formats/from/ics.rs b/crates/nu-command/src/formats/from/ics.rs index 5a3990329..6d364d6b6 100644 --- a/crates/nu-command/src/formats/from/ics.rs +++ b/crates/nu-command/src/formats/from/ics.rs @@ -5,8 +5,8 @@ use indexmap::map::IndexMap; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ - Category, Config, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, - Spanned, Type, Value, + Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Spanned, Type, + Value, }; use std::io::BufReader; @@ -30,14 +30,13 @@ impl Command for FromIcs { fn run( &self, - engine_state: &EngineState, + _engine_state: &EngineState, _stack: &mut Stack, call: &Call, input: PipelineData, ) -> Result { let head = call.head; - let config = engine_state.get_config(); - from_ics(input, head, config) + from_ics(input, head) } fn examples(&self) -> Vec { @@ -94,8 +93,8 @@ END:VCALENDAR' | from ics", } } -fn from_ics(input: PipelineData, head: Span, config: &Config) -> Result { - let input_string = input.collect_string("", config)?; +fn from_ics(input: PipelineData, head: Span) -> Result { + let (input_string, metadata) = input.collect_string_strict(head)?; let input_string = input_string .lines() @@ -124,7 +123,7 @@ fn from_ics(input: PipelineData, head: Span, config: &Config) -> Result Value { diff --git a/crates/nu-command/src/formats/from/ini.rs b/crates/nu-command/src/formats/from/ini.rs index 7c31d342f..d0c7318d4 100644 --- a/crates/nu-command/src/formats/from/ini.rs +++ b/crates/nu-command/src/formats/from/ini.rs @@ -2,8 +2,7 @@ use indexmap::map::IndexMap; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ - Category, Config, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Type, - Value, + Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Type, Value, }; #[derive(Clone)] @@ -53,14 +52,13 @@ b=2' | from ini", fn run( &self, - engine_state: &EngineState, + _engine_state: &EngineState, _stack: &mut Stack, call: &Call, input: PipelineData, ) -> Result { let head = call.head; - let config = engine_state.get_config(); - from_ini(input, head, config) + from_ini(input, head) } } @@ -90,11 +88,11 @@ pub fn from_ini_string_to_value(s: String, span: Span) -> Result Result { - let concat_string = input.collect_string("", config)?; +fn from_ini(input: PipelineData, head: Span) -> Result { + let (concat_string, metadata) = input.collect_string_strict(head)?; match from_ini_string_to_value(concat_string, head) { - Ok(x) => Ok(x.into_pipeline_data()), + Ok(x) => Ok(x.into_pipeline_data_with_metadata(metadata)), Err(other) => Err(other), } } diff --git a/crates/nu-command/src/formats/from/json.rs b/crates/nu-command/src/formats/from/json.rs index 1ddaecb6e..5d551c407 100644 --- a/crates/nu-command/src/formats/from/json.rs +++ b/crates/nu-command/src/formats/from/json.rs @@ -76,11 +76,10 @@ impl Command for FromJson { input: PipelineData, ) -> Result { let span = call.head; - let config = engine_state.get_config(); - let string_input = input.collect_string("", config)?; + let (string_input, metadata) = input.collect_string_strict(span)?; if string_input.is_empty() { - return Ok(PipelineData::new(span)); + return Ok(PipelineData::new_with_metadata(metadata, span)); } // TODO: turn this into a structured underline of the nu_json error @@ -98,9 +97,11 @@ impl Command for FromJson { } }) .collect(); - Ok(converted_lines.into_pipeline_data(engine_state.ctrlc.clone())) + Ok(converted_lines + .into_pipeline_data_with_metadata(metadata, engine_state.ctrlc.clone())) } else { - Ok(convert_string_to_value(string_input, span)?.into_pipeline_data()) + Ok(convert_string_to_value(string_input, span)? + .into_pipeline_data_with_metadata(metadata)) } } } diff --git a/crates/nu-command/src/formats/from/nuon.rs b/crates/nu-command/src/formats/from/nuon.rs index 7c0cab61b..bd6e12dcc 100644 --- a/crates/nu-command/src/formats/from/nuon.rs +++ b/crates/nu-command/src/formats/from/nuon.rs @@ -68,14 +68,13 @@ impl Command for FromNuon { fn run( &self, - engine_state: &EngineState, + _engine_state: &EngineState, _stack: &mut Stack, call: &Call, input: PipelineData, ) -> Result { let head = call.head; - let config = engine_state.get_config(); - let string_input = input.collect_string("", config)?; + let (string_input, metadata) = input.collect_string_strict(head)?; let engine_state = EngineState::new(); @@ -183,7 +182,7 @@ impl Command for FromNuon { let result = convert_to_value(expr, head, &string_input); match result { - Ok(result) => Ok(result.into_pipeline_data()), + Ok(result) => Ok(result.into_pipeline_data_with_metadata(metadata)), Err(err) => Err(ShellError::GenericError( "error when loading nuon text".into(), "could not load nuon text".into(), diff --git a/crates/nu-command/src/formats/from/ssv.rs b/crates/nu-command/src/formats/from/ssv.rs index ff34b1fea..ff655420d 100644 --- a/crates/nu-command/src/formats/from/ssv.rs +++ b/crates/nu-command/src/formats/from/ssv.rs @@ -268,7 +268,6 @@ fn from_ssv( call: &Call, input: PipelineData, ) -> Result { - let config = engine_state.get_config(); let name = call.head; let noheaders = call.has_flag("noheaders"); @@ -276,7 +275,7 @@ fn from_ssv( let minimum_spaces: Option> = call.get_flag(engine_state, stack, "minimum-spaces")?; - let concat_string = input.collect_string("", config)?; + let (concat_string, metadata) = input.collect_string_strict(name)?; let split_at = match minimum_spaces { Some(number) => number.item, None => DEFAULT_MINIMUM_SPACES, @@ -284,7 +283,7 @@ fn from_ssv( Ok( from_ssv_string_to_value(&concat_string, noheaders, aligned_columns, split_at, name) - .into_pipeline_data(), + .into_pipeline_data_with_metadata(metadata), ) } diff --git a/crates/nu-command/src/formats/from/toml.rs b/crates/nu-command/src/formats/from/toml.rs index 2c52fa0d7..d4652f7c3 100644 --- a/crates/nu-command/src/formats/from/toml.rs +++ b/crates/nu-command/src/formats/from/toml.rs @@ -69,16 +69,15 @@ b = [1, 2]' | from toml", fn run( &self, - engine_state: &EngineState, + __engine_state: &EngineState, _stack: &mut Stack, call: &Call, input: PipelineData, ) -> Result { let span = call.head; - let config = engine_state.get_config(); - let mut string_input = input.collect_string("", config)?; + let (mut string_input, metadata) = input.collect_string_strict(span)?; string_input.push('\n'); - Ok(convert_string_to_value(string_input, span)?.into_pipeline_data()) + Ok(convert_string_to_value(string_input, span)?.into_pipeline_data_with_metadata(metadata)) } } diff --git a/crates/nu-command/src/formats/from/tsv.rs b/crates/nu-command/src/formats/from/tsv.rs index 295daa35f..40e9dbfd0 100644 --- a/crates/nu-command/src/formats/from/tsv.rs +++ b/crates/nu-command/src/formats/from/tsv.rs @@ -106,15 +106,7 @@ fn from_tsv( let trim: Option = call.get_flag(engine_state, stack, "trim")?; let trim = trim_from_str(trim)?; - from_delimited_data( - noheaders, - no_infer, - '\t', - trim, - input, - name, - engine_state.get_config(), - ) + from_delimited_data(noheaders, no_infer, '\t', trim, input, name) } #[cfg(test)] diff --git a/crates/nu-command/src/formats/from/url.rs b/crates/nu-command/src/formats/from/url.rs index 2b89956d8..3bb1ea5d6 100644 --- a/crates/nu-command/src/formats/from/url.rs +++ b/crates/nu-command/src/formats/from/url.rs @@ -1,8 +1,6 @@ use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; -use nu_protocol::{ - Category, Config, Example, PipelineData, ShellError, Signature, Span, Type, Value, -}; +use nu_protocol::{Category, Example, PipelineData, ShellError, Signature, Span, Type, Value}; #[derive(Clone)] pub struct FromUrl; @@ -24,14 +22,13 @@ impl Command for FromUrl { fn run( &self, - engine_state: &EngineState, + _engine_state: &EngineState, _stack: &mut Stack, call: &Call, input: PipelineData, ) -> Result { let head = call.head; - let config = engine_state.get_config(); - from_url(input, head, config) + from_url(input, head) } fn examples(&self) -> Vec { @@ -57,8 +54,8 @@ impl Command for FromUrl { } } -fn from_url(input: PipelineData, head: Span, config: &Config) -> Result { - let concat_string = input.collect_string("", config)?; +fn from_url(input: PipelineData, head: Span) -> Result { + let (concat_string, metadata) = input.collect_string_strict(head)?; let result = serde_urlencoded::from_str::>(&concat_string); @@ -77,7 +74,7 @@ fn from_url(input: PipelineData, head: Span, config: &Config) -> Result Err(ShellError::UnsupportedInput( diff --git a/crates/nu-command/src/formats/from/vcf.rs b/crates/nu-command/src/formats/from/vcf.rs index cb42c6e42..baaf328fe 100644 --- a/crates/nu-command/src/formats/from/vcf.rs +++ b/crates/nu-command/src/formats/from/vcf.rs @@ -4,8 +4,8 @@ use indexmap::map::IndexMap; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ - Category, Config, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, - Spanned, Type, Value, + Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Spanned, Type, + Value, }; #[derive(Clone)] @@ -28,14 +28,13 @@ impl Command for FromVcf { fn run( &self, - engine_state: &EngineState, + _engine_state: &EngineState, _stack: &mut Stack, call: &Call, input: PipelineData, ) -> Result { let head = call.head; - let config = engine_state.get_config(); - from_vcf(input, head, config) + from_vcf(input, head) } fn examples(&self) -> Vec { @@ -125,8 +124,8 @@ END:VCARD' | from vcf", } } -fn from_vcf(input: PipelineData, head: Span, config: &Config) -> Result { - let input_string = input.collect_string("", config)?; +fn from_vcf(input: PipelineData, head: Span) -> Result { + let (input_string, metadata) = input.collect_string_strict(head)?; let input_string = input_string .lines() @@ -153,7 +152,7 @@ fn from_vcf(input: PipelineData, head: Span, config: &Config) -> Result Value { diff --git a/crates/nu-command/src/formats/from/xml.rs b/crates/nu-command/src/formats/from/xml.rs index 41fe153b1..f973ae10d 100644 --- a/crates/nu-command/src/formats/from/xml.rs +++ b/crates/nu-command/src/formats/from/xml.rs @@ -2,8 +2,8 @@ use indexmap::map::IndexMap; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ - Category, Config, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, - Spanned, Type, Value, + Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Spanned, Type, + Value, }; #[derive(Clone)] @@ -26,14 +26,13 @@ impl Command for FromXml { fn run( &self, - engine_state: &EngineState, + _engine_state: &EngineState, _stack: &mut Stack, call: &Call, input: PipelineData, ) -> Result { let head = call.head; - let config = engine_state.get_config(); - from_xml(input, head, config) + from_xml(input, head) } fn examples(&self) -> Vec { @@ -180,11 +179,11 @@ pub fn from_xml_string_to_value(s: String, span: Span) -> Result Result { - let concat_string = input.collect_string("", config)?; +fn from_xml(input: PipelineData, head: Span) -> Result { + let (concat_string, metadata) = input.collect_string_strict(head)?; match from_xml_string_to_value(concat_string, head) { - Ok(x) => Ok(x.into_pipeline_data()), + Ok(x) => Ok(x.into_pipeline_data_with_metadata(metadata)), _ => Err(ShellError::UnsupportedInput( "Could not parse string as xml".to_string(), head, diff --git a/crates/nu-command/src/formats/from/yaml.rs b/crates/nu-command/src/formats/from/yaml.rs index 3f07ae5ef..d94150000 100644 --- a/crates/nu-command/src/formats/from/yaml.rs +++ b/crates/nu-command/src/formats/from/yaml.rs @@ -2,8 +2,8 @@ use itertools::Itertools; use nu_protocol::ast::Call; use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ - Category, Config, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, - Spanned, Type, Value, + Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Spanned, Type, + Value, }; use serde::de::Deserialize; use std::collections::HashMap; @@ -32,14 +32,13 @@ impl Command for FromYaml { fn run( &self, - engine_state: &EngineState, + _engine_state: &EngineState, _stack: &mut Stack, call: &Call, input: PipelineData, ) -> Result { let head = call.head; - let config = engine_state.get_config(); - from_yaml(input, head, config) + from_yaml(input, head) } } @@ -61,14 +60,13 @@ impl Command for FromYml { fn run( &self, - engine_state: &EngineState, + _engine_state: &EngineState, _stack: &mut Stack, call: &Call, input: PipelineData, ) -> Result { let head = call.head; - let config = engine_state.get_config(); - from_yaml(input, head, config) + from_yaml(input, head) } fn examples(&self) -> Vec { @@ -215,11 +213,11 @@ pub fn get_examples() -> Vec { ] } -fn from_yaml(input: PipelineData, head: Span, config: &Config) -> Result { - let concat_string = input.collect_string("", config)?; +fn from_yaml(input: PipelineData, head: Span) -> Result { + let (concat_string, metadata) = input.collect_string_strict(head)?; match from_yaml_string_to_value(concat_string, head) { - Ok(x) => Ok(x.into_pipeline_data()), + Ok(x) => Ok(x.into_pipeline_data_with_metadata(metadata)), Err(other) => Err(other), } } @@ -227,6 +225,7 @@ fn from_yaml(input: PipelineData, head: Span, config: &Config) -> Result Result<(String, Option), ShellError> { + match self { + PipelineData::Value(Value::String { val, .. }, metadata) => Ok((val, metadata)), + PipelineData::Value(val, _) => { + Err(ShellError::TypeMismatch("string".into(), val.span()?)) + } + PipelineData::ListStream(_, _) => Err(ShellError::TypeMismatch("string".into(), span)), + PipelineData::ExternalStream { + stdout: None, + metadata, + .. + } => Ok((String::new(), metadata)), + PipelineData::ExternalStream { + stdout: Some(stdout), + metadata, + .. + } => Ok((stdout.into_string()?.item, metadata)), + } + } + pub fn follow_cell_path( self, cell_path: &[PathMember], @@ -596,7 +623,10 @@ impl Iterator for PipelineIterator { pub trait IntoPipelineData { fn into_pipeline_data(self) -> PipelineData; - fn into_pipeline_data_with_metadata(self, metadata: PipelineMetadata) -> PipelineData; + fn into_pipeline_data_with_metadata( + self, + metadata: impl Into>, + ) -> PipelineData; } impl IntoPipelineData for V @@ -606,8 +636,11 @@ where fn into_pipeline_data(self) -> PipelineData { PipelineData::Value(self.into(), None) } - fn into_pipeline_data_with_metadata(self, metadata: PipelineMetadata) -> PipelineData { - PipelineData::Value(self.into(), Some(metadata)) + fn into_pipeline_data_with_metadata( + self, + metadata: impl Into>, + ) -> PipelineData { + PipelineData::Value(self.into(), metadata.into()) } } @@ -615,7 +648,7 @@ pub trait IntoInterruptiblePipelineData { fn into_pipeline_data(self, ctrlc: Option>) -> PipelineData; fn into_pipeline_data_with_metadata( self, - metadata: PipelineMetadata, + metadata: impl Into>, ctrlc: Option>, ) -> PipelineData; } @@ -638,7 +671,7 @@ where fn into_pipeline_data_with_metadata( self, - metadata: PipelineMetadata, + metadata: impl Into>, ctrlc: Option>, ) -> PipelineData { PipelineData::ListStream( @@ -646,7 +679,7 @@ where stream: Box::new(self.into_iter().map(Into::into)), ctrlc, }, - Some(metadata), + metadata.into(), ) } }