Skip to content

Commit

Permalink
fix: consider value type when escaping and unescaping
Browse files Browse the repository at this point in the history
this fixes #104 (introduced in 0.16.2) where URL properties got
unnecessarily escaped
  • Loading branch information
hoodie committed Aug 13, 2024
1 parent a57ce3e commit c65fe71
Show file tree
Hide file tree
Showing 7 changed files with 289 additions and 100 deletions.
2 changes: 1 addition & 1 deletion src/components/alarm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use super::*;
/// When it is time for the Alarm to occur we have to define what is actually supposed to happen.
/// The RFC5545 know three different actions, two of which are currently implemented.
///
/// 1. Display
/// 1. Display
/// 2. Audio
/// 3. Email (not yet implemented)
///
Expand Down
3 changes: 1 addition & 2 deletions src/components/date_time.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#![allow(dead_code, unused)]
use std::str::FromStr;

use chrono::{DateTime, Duration, NaiveDate, NaiveDateTime, Offset, TimeZone as _, Utc};
use chrono::{DateTime, Duration, NaiveDate, NaiveDateTime, TimeZone as _, Utc};

use crate::{Property, ValueType};

Expand Down
4 changes: 3 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ mod components;
#[cfg(feature = "parser")]
pub mod parser;
mod properties;
mod value_types;

pub use crate::{
calendar::{Calendar, CalendarComponent},
Expand All @@ -108,7 +109,8 @@ pub use crate::{
date_time::{CalendarDateTime, DatePerhapsTime},
Component, Event, EventLike, Todo, Venue,
},
properties::{Class, EventStatus, Parameter, Property, TodoStatus, ValueType},
properties::{Class, EventStatus, Parameter, Property, TodoStatus},
value_types::ValueType,
};

#[cfg(feature = "chrono-tz")]
Expand Down
9 changes: 9 additions & 0 deletions src/parser/parsed_string.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::borrow::Cow;

use crate::ValueType;

/// A zero-copy string parsed from an iCal input.
#[derive(Debug, Eq, Clone)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
Expand All @@ -26,6 +28,13 @@ impl ParseString<'_> {
}

impl<'a> ParseString<'a> {
pub fn unescape_by_value_type(self, value_type: ValueType) -> ParseString<'a> {
match value_type {
ValueType::Text => self.unescape_text(),
_ => self,
}
}

pub fn unescape_text(self) -> ParseString<'a> {
if self.0.contains(r#"\\"#)
|| self.0.contains(r#"\,"#)
Expand Down
110 changes: 95 additions & 15 deletions src/parser/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{
str::FromStr,
};

use crate::{parser::utils::valid_key_sequence_cow, properties::fold_line};
use crate::{parser::utils::valid_key_sequence_cow, properties::fold_line, value_types::ValueType};

use super::{
parameters::{parameters, Parameter},
Expand Down Expand Up @@ -124,7 +124,7 @@ impl FromStr for crate::Property {
}

#[test]
fn test_property() {
fn properties() {
assert_parser!(
property,
"KEY:VALUE\n",
Expand Down Expand Up @@ -164,31 +164,97 @@ fn test_property() {
params: vec![Parameter::new_ref("foo", Some("bar"))]
}
);
}

#[test]
fn unescape_text() {
assert_eq!(
"DESCRIPTION:https\\://example.com\n"
.parse::<crate::Property>()
.map(|p| p.val)
.unwrap(),
"https://example.com"
);

assert_parser!(
property,
"DESCRIPTION:https\\://example.com\n",
Property {
name: "DESCRIPTION".into(),
val: "https://example.com".into(),
params: Default::default()
}
);
}

#[test]
fn property_escape_url_as_url() {
assert_eq!(
"URL:https://example.com\n"
.parse::<crate::Property>()
.map(|p| p.val)
.unwrap(),
"https://example.com"
);

assert_parser!(
property,
"URL:https://example.com\n",
Property {
name: "URL".into(),
val: "https://example.com".into(),
params: Default::default()
}
);
}

// TODO: newlines followed by spaces must be ignored
#[test]
fn property_escape_description_as_text() {
assert_parser!(
property,
"KEY4;foo=bar:VALUE\\n newline separated\n",
"DESCRIPTION;foo=bar:VALUE\\n newline separated\n",
Property {
name: "KEY4".into(),
name: "DESCRIPTION".into(),
val: "VALUE\n newline separated".into(),
params: vec![Parameter::new_ref("foo", Some("bar"))]
}
);
}

#[test]
fn property_escape_by_value_param() {
assert_parser!(
property,
"KEY3;foo=bar:VALUE\\n newline separated\n",
"FOO;VALUE=TEXT:VALUE\\n newline separated\n",
Property {
name: "KEY3".into(),
name: "FOO".into(),
val: "VALUE\n newline separated".into(),
params: vec![Parameter::new_ref("foo", Some("bar"))]
params: vec![Parameter::new_ref("VALUE", Some("TEXT"))]
}
);

assert_parser!(
property,
"FOO_AS_TEXT;VALUE=TEXT:https\\://example.com\n",
Property {
name: "FOO_AS_TEXT".into(),
val: "https://example.com".into(),
params: vec![Parameter::new_ref("VALUE", Some("TEXT"))]
}
);
assert_parser!(
property,
"FOO_AS_URL;VALUE=URL:https\\://example.com\n",
Property {
name: "FOO_AS_URL".into(),
val: "https\\://example.com".into(),
params: vec![Parameter::new_ref("VALUE", Some("URL"))]
}
);
}

#[test]
fn test_property_with_dash() {
fn property_with_dash() {
assert_parser!(
property,
"X-HOODIE-KEY:VALUE\n",
Expand Down Expand Up @@ -318,8 +384,7 @@ pub fn property<'a, E: ParseError<&'a str> + ContextError<&'a str>>(
// this is for single line prop parsing, just so I can leave off the '\n'
take_while(|_| true),
))
.map(ParseString::from)
.map(ParseString::unescape_text),
.map(ParseString::from),
),
),
context(
Expand All @@ -329,10 +394,25 @@ pub fn property<'a, E: ParseError<&'a str> + ContextError<&'a str>>(
)),
opt(line_ending),
))
.map(|(((key, params), val), _)| Property {
name: key,
val,
params,
.map(|(((name, params), val), _)| {
let value_type = params
.iter()
.find(|param| param.key == "VALUE")
.and_then(|Parameter { val, .. }| val.as_ref())
.and_then(|typ| ValueType::from_str(typ.as_str()).ok())
.or_else(|| ValueType::by_name(name.as_str()));

if let Some(value_type) = value_type {
eprintln!("escaping for {value_type:?}");
Property {
name,
val: val.unescape_by_value_type(value_type),
params,
}
} else {
eprintln!("NOT escaping");
Property { name, val, params }
}
})),
)
.parse(input)
Expand Down
124 changes: 43 additions & 81 deletions src/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ use std::{
collections::HashMap,
fmt::{self, Write},
mem,
str::FromStr,
};

use crate::value_types::ValueType;

#[derive(Clone, Debug, PartialEq, Eq)]
/// key-value pairs inside of `Property`s
pub struct Parameter {
Expand Down Expand Up @@ -89,7 +92,10 @@ impl Property {

/// Returns the `VALUE` parameter, if any is specified.
pub fn value_type(&self) -> Option<ValueType> {
ValueType::from_str(&self.params.get("VALUE")?.val)
self.params
.get("VALUE")
.and_then(|p| ValueType::from_str(&p.val).ok())
.or_else(|| ValueType::by_name(self.key()))
}

// /// Returns the value as a certain type
Expand Down Expand Up @@ -171,7 +177,17 @@ impl Property {
for Parameter { key, val } in self.params.values() {
write!(line, ";{}={}", key, Self::quote_if_contains_colon(val))?;
}
write!(line, ":{}", Self::escape_text(&self.val))?;
let value_type = self.value_type();
eprintln!(
"{}:{} value type of {key} = {value_type:?}",
file!(),
line!(),
key = self.key
);
match value_type {
Some(ValueType::Text) => write!(line, ":{}", Self::escape_text(&self.val))?,
_ => write!(line, ":{}", self.val)?,
}
write_crlf!(out, "{}", fold_line(&line))?;
Ok(())
}
Expand Down Expand Up @@ -224,85 +240,6 @@ impl From<Class> for Property {
}
}

/// see 8.3.4. [Value Data Types Registry](https://tools.ietf.org/html/rfc5545#section-8.3.4)
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum ValueType {
/// [`Binary`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.1)
Binary,
/// [`Boolean`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.2)
Boolean,
/// [`CalAddress`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.3)
CalAddress,
/// [`Date`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.4)
Date,
/// [`DateTime`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.5)
DateTime,
/// [`Duration`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.6)
Duration,
/// [`Float`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.7)
Float,
/// [`Integer`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.8)
Integer,
/// [`Period`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.9)
Period,
/// [`Recur`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.10)
Recur,
/// [`Text`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.11)
Text,
/// [`Time`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.12)
Time,
/// [`Uri`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.13)
Uri,
/// [`UtcOffset`](https://datatracker.ietf.org/doc/html/rfc5545#section-3.3.14)
UtcOffset,
}

impl ValueType {
pub(crate) fn from_str(s: &str) -> Option<Self> {
match s {
"BINARY" => Some(Self::Binary),
"BOOLEAN" => Some(Self::Boolean),
"CAL-ADDRESS" => Some(Self::CalAddress),
"DATE" => Some(Self::Date),
"DATE-TIME" => Some(Self::DateTime),
"DURATION" => Some(Self::Duration),
"FLOAT" => Some(Self::Float),
"INTEGER" => Some(Self::Integer),
"PERIOD" => Some(Self::Period),
"RECUR" => Some(Self::Recur),
"TEXT" => Some(Self::Text),
"TIME" => Some(Self::Time),
"URI" => Some(Self::Uri),
"UTC-OFFSET" => Some(Self::UtcOffset),
_ => None,
}
}
}

impl From<ValueType> for Parameter {
fn from(val: ValueType) -> Self {
Parameter {
key: String::from("VALUE"),
val: String::from(match val {
ValueType::Binary => "BINARY",
ValueType::Boolean => "BOOLEAN",
ValueType::CalAddress => "CAL-ADDRESS",
ValueType::Date => "DATE",
ValueType::DateTime => "DATE-TIME",
ValueType::Duration => "DURATION",
ValueType::Float => "FLOAT",
ValueType::Integer => "INTEGER",
ValueType::Period => "PERIOD",
ValueType::Recur => "RECUR",
ValueType::Text => "TEXT",
ValueType::Time => "TIME",
ValueType::Uri => "URI",
ValueType::UtcOffset => "UTC-OFFSET",
}),
}
}
}

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
/// Encodes the status of an `Event`
/// [RFC 5545, Section 3.8.1.11](https://datatracker.ietf.org/doc/html/rfc5545#section-3.8.1.11)
Expand Down Expand Up @@ -375,6 +312,31 @@ impl From<EventStatus> for Property {
}
}

// TODO: why do we add this?
impl From<ValueType> for Parameter {
fn from(val: ValueType) -> Self {
Parameter {
key: String::from("VALUE"),
val: String::from(match val {
ValueType::Binary => "BINARY",
ValueType::Boolean => "BOOLEAN",
ValueType::CalAddress => "CAL-ADDRESS",
ValueType::Date => "DATE",
ValueType::DateTime => "DATE-TIME",
ValueType::Duration => "DURATION",
ValueType::Float => "FLOAT",
ValueType::Integer => "INTEGER",
ValueType::Period => "PERIOD",
ValueType::Recur => "RECUR",
ValueType::Text => "TEXT",
ValueType::Time => "TIME",
ValueType::Uri => "URI",
ValueType::UtcOffset => "UTC-OFFSET",
}),
}
}
}

impl From<TodoStatus> for Property {
fn from(val: TodoStatus) -> Self {
Property::new_pre_alloc(
Expand Down
Loading

0 comments on commit c65fe71

Please sign in to comment.