Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split armor at empty line #209

Closed
toberndo opened this issue Apr 9, 2014 · 7 comments · Fixed by #225
Closed

Split armor at empty line #209

toberndo opened this issue Apr 9, 2014 · 7 comments · Fixed by #225

Comments

@toberndo
Copy link
Member

toberndo commented Apr 9, 2014

The empty line that separates armor header from body can contain whitespace-y characters. See #193

Currently we split at: /^\s*\n/m;

Suggestions:

/^\s_?\n/m;
/^[\t ]\n/m;
/^[\t ]
$/;
/^\s_$/;

@Robert-Nelson
Copy link
Contributor

/^\s*?\n/m; has the same effect as the existing code so it won't work. The other three that I gave should all work.

----- Original Message -----

| From: "Thomas Oberndörfer" notifications@github.com
| To: "openpgpjs/openpgpjs" openpgpjs@noreply.github.com
| Sent: Wednesday, April 9, 2014 9:58:33 AM
| Subject: [openpgpjs] Split armor at empty line (#209)

| The empty line that separates armor header from body can contain whitespace-y
| characters. See #193

| Currently we split at: /^\s*\n/m;

| Suggestions:

| /^\s_?\n/m;
| /^[\t ] \n/m;
| /^[\t ] $/;
| /^\s_$/;

| —
| Reply to this email directly or view it on GitHub .

@toberndo
Copy link
Member Author

toberndo commented Apr 9, 2014

I checked:

"\f\r\t\v​\u00a0\u1680​\u180e\u2000​\u2001\u2002​\u2003\u2004​\u2005\u2006​\u2007\u2008\u2009\u200a​\u2028\u2029​​\u202f\u205f​\u3000".replace(/ /g,'')

And it doesn't replace other whitespace characters.

So I think /^[\t ]\n/m; would only filer out spaces (0x20) and tabs but not other whitespace characters.

@toberndo
Copy link
Member Author

toberndo commented Apr 9, 2014

@Robert-Nelson
Copy link
Contributor

I think that would work but is overly complex with the multiple negatives.

I think the simplest is:

/^[ \t]*\n/m

I remember now why I used the "m" modifier, I wanted the trailing \n included in the match, so some of my other suggestions earlier weren't right.

----- Original Message -----

| From: "Thomas Oberndörfer" notifications@github.com
| To: "openpgpjs/openpgpjs" openpgpjs@noreply.github.com
| Cc: "Robert Nelson" robertn@the-nelsons.org
| Sent: Wednesday, April 9, 2014 11:09:57 AM
| Subject: Re: [openpgpjs] Split armor at empty line (#209)

| @Robert-Nelson how about /^[^\S\n]*\n/m; from
| http://stackoverflow.com/questions/3469080/how-do-i-match-whitespace-but-not-newlines
| ?

| —
| Reply to this email directly or view it on GitHub .

@Robert-Nelson
Copy link
Contributor

Ah, forgot about damn unicode, in that case go with your suggestion.

----- Original Message -----

| From: "Thomas Oberndörfer" notifications@github.com
| To: "openpgpjs/openpgpjs" openpgpjs@noreply.github.com
| Cc: "Robert Nelson" robertn@the-nelsons.org
| Sent: Wednesday, April 9, 2014 10:24:42 AM
| Subject: Re: [openpgpjs] Split armor at empty line (#209)

| I checked:
| "\f\r\t\v​\u00a0\u1680​\u180e\u2000​\u2001\u2002​\u2003\u2004​\u2005\u2006​\u2007\u2008\u2009\u200a​\u2028\u2029​​\u202f\u205f​\u3000".replace(/
| /g,'')

| And it doesn't replace other whitespace characters.

| So I think /^[\t ]\n/m; would only filer out spaces (0x20) and tabs but not
| other whitespace characters.

| —
| Reply to this email directly or view it on GitHub .

@Robert-Nelson
Copy link
Contributor

Btw RFC 4880 says empty line so I'm not sure that technically any whitespace chars are valid. :-)

----- Original Message -----

| From: "Robert B. Nelson" robertn@the-nelsons.org
| To: "openpgpjs/openpgpjs"
| reply@reply.github.com
| Cc: "openpgpjs/openpgpjs" openpgpjs@noreply.github.com
| Sent: Wednesday, April 9, 2014 12:47:46 PM
| Subject: Re: [openpgpjs] Split armor at empty line (#209)

| Ah, forgot about damn unicode, in that case go with your suggestion.

| ----- Original Message -----

| | From: "Thomas Oberndörfer" notifications@github.com
|
| | To: "openpgpjs/openpgpjs" openpgpjs@noreply.github.com
|
| | Cc: "Robert Nelson" robertn@the-nelsons.org
|
| | Sent: Wednesday, April 9, 2014 10:24:42 AM
|
| | Subject: Re: [openpgpjs] Split armor at empty line (#209)
|

| | I checked:
|
| | "\f\r\t\v​\u00a0\u1680​\u180e\u2000​\u2001\u2002​\u2003\u2004​\u2005\u2006​\u2007\u2008\u2009\u200a​\u2028\u2029​​\u202f\u205f​\u3000".replace(/
| | /g,'')
|

| | And it doesn't replace other whitespace characters.
|

| | So I think /^[\t ]\n/m; would only filer out spaces (0x20) and tabs but not
| | other whitespace characters.
|

| | —
|
| | Reply to this email directly or view it on GitHub .
|

@toberndo
Copy link
Member Author

Created pull request for this issue: #225

@tanx tanx closed this as completed in #225 May 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants