From 6ee1d67e380ce7808ca5748263aa3e2829bf64a0 Mon Sep 17 00:00:00 2001
From: Simone Gotti <simone.gotti@gmail.com>
Date: Tue, 29 Mar 2016 14:39:18 +0200
Subject: [PATCH] spec: Define image format string

Add an "extra" spec section for image format string

This format is actually used by the appc spec golang reference
implementation and other tools (like rkt) but it has not been defined.

There're also various unclear cases in the actual implementationt that
this definition tries to clarify, but this is going to change some
behaviors.

The main problem is related to the label values. Since labels are
strings, they can contain any character (also special character like
carriage return, tabs etc...).

With the current implementation there are two main issues relatd to
label value being strings:
* label value containing `,` `:` and `=` cannot be used inside the image format
string. There's the need to find a way to escape them.

* Many special character (like non printable characters) are difficult
to represent in the actual format.

Instead name and label name are of type ACIIdentifier, so they can
contain only a specific set of character and in this set `,`, `:` and
`=` aren't included.

Instead of adding another ad hoc escape rule this spec requires that the
labels value must be URL escaped (percent encoding) with UTF-8 encoding.

This will break some current implementations:

For example rkt stage1 has a version like `1.2.1+gite568957-dirty` and
with the current image format parsing it works. Now, with the need to URL
escape the labels values it should be expressed as
`1.2.1%2Bgite568957-dirty` (since passing `1.2.1+gite568957-dirty` will
be interpreted as `1.2.1 gite568957-dirty`).
---
 discovery/parse.go         |  6 +++++-
 discovery/parse_test.go    |  5 +++--
 spec/extras/imageformat.md | 26 ++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 3 deletions(-)
 create mode 100644 spec/extras/imageformat.md

diff --git a/discovery/parse.go b/discovery/parse.go
index 7b9d574a..9ada039e 100644
--- a/discovery/parse.go
+++ b/discovery/parse.go
@@ -79,7 +79,11 @@ func NewAppFromString(app string) (*App, error) {
 		if err != nil {
 			return nil, err
 		}
-		labels[*labelName] = val[0]
+		labelValue, err := url.QueryUnescape(val[0])
+		if err != nil {
+			return nil, err
+		}
+		labels[*labelName] = labelValue
 	}
 	a, err := NewApp(name, labels)
 	if err != nil {
diff --git a/discovery/parse_test.go b/discovery/parse_test.go
index 61bd4a99..88b42e67 100644
--- a/discovery/parse_test.go
+++ b/discovery/parse_test.go
@@ -53,13 +53,14 @@ func TestNewAppFromString(t *testing.T) {
 			false,
 		},
 		{
-			"example.com/app:1.2.3,special=!*'();@&+$/?#[],channel=beta",
+			// URL escaped !*'();@&+$/?#[]
+			"example.com/app:1.2.3,special=%21%2A%27%28%29%3B%40%26%2B%24%2F%3F%23%5B%5D%C2%BC%C2%B5%C3%9F,channel=beta",
 
 			&App{
 				Name: "example.com/app",
 				Labels: map[types.ACIdentifier]string{
 					"version": "1.2.3",
-					"special": "!*'();@&+$/?#[]",
+					"special": "!*'();@&+$/?#[]¼µß",
 					"channel": "beta",
 				},
 			},
diff --git a/spec/extras/imageformat.md b/spec/extras/imageformat.md
new file mode 100644
index 00000000..ad46e8fd
--- /dev/null
+++ b/spec/extras/imageformat.md
@@ -0,0 +1,26 @@
+## Image format string
+
+This specification defines an image format string to use to reference an image. It can be used by discovery or by an ACI to let the user define the image to discovery, execute etc...
+
+An image format string consists of an image name and a set of labels with this format:
+
+```
+name[:versionvalue][,labelname=labelvalue][,labelname=labelvalue]...
+```
+
+Where:
+* name (string, required) a human-readable name for an App Container Image (string, restricted to the AC Identifier formatting)
+* versionvalue (string, optional) is a shortcut to define a version label. Writing `name:versionvalue` is the same of writing `name,version=versionvalue`
+* labelname=labelvalue (optional) A label definition, must have two key-value pairs: *labelname* is restricted to the AC Identifier formatting and *labelvalue* is an arbitrary string.
+
+Since *labelvalue* (and *versionvalue*) can be an arbitrary string it MUST be URL escaped (percent encoded) (http://tools.ietf.org/html/rfc3986#section-2) with UTF-8 encoding.
+
+
+### Examples
+
+An image format string for an image with name "example.com/reduce-worker" with version "0.1.0+gitabcdef" will be:
+
+`example.com/reduce-worker:0.1.0%2Bgitabcdef` or
+`example.com/reduce-worker,version=0.1.0%2Bgitabcdef`
+
+Note that the `+` in the *versionvalue* has been replace by `%2B` since the label value MUST be URL encoded.