public inbox for cygwin-apps@cygwin.com
 help / color / mirror / Atom feed
* [PATCH setup] Don't write garbage into the "extrakeys" user setting
@ 2017-11-27 21:36 Ken Brown
  2017-11-27 22:20 ` Ken Brown
  2017-11-27 22:26 ` Jon Turney
  0 siblings, 2 replies; 6+ messages in thread
From: Ken Brown @ 2017-11-27 21:36 UTC (permalink / raw)
  To: cygwin-apps

The ExtraKeysSetting destructor called UserSettings::set() on a string
that was terminated by LF instead of NUL.  This led to garbage at the
end of the "extrakeys" setting that was written into setup.rc.  Fix
this by replacing the final LF by NUL before calling set().
---
 KeysSetting.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/KeysSetting.cc b/KeysSetting.cc
index a21c3c4..19c6df3 100644
--- a/KeysSetting.cc
+++ b/KeysSetting.cc
@@ -46,7 +46,11 @@ ExtraKeysSetting::ExtraKeysSetting ():
 ExtraKeysSetting::~ExtraKeysSetting ()
 {
   if (keybuffer)
-    UserSettings::instance().set ("extrakeys", keybuffer);
+    {
+      // Replace final LF with NUL.
+      keybuffer[bufsize - 1] = '\0';
+      UserSettings::instance().set ("extrakeys", keybuffer);
+    }
 }
 
 void
-- 
2.15.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH setup] Don't write garbage into the "extrakeys" user setting
  2017-11-27 21:36 [PATCH setup] Don't write garbage into the "extrakeys" user setting Ken Brown
@ 2017-11-27 22:20 ` Ken Brown
  2017-11-28  2:14   ` Ken Brown
  2017-11-27 22:26 ` Jon Turney
  1 sibling, 1 reply; 6+ messages in thread
From: Ken Brown @ 2017-11-27 22:20 UTC (permalink / raw)
  To: cygwin-apps

On 11/27/2017 4:35 PM, Ken Brown wrote:
> The ExtraKeysSetting destructor called UserSettings::set() on a string
> that was terminated by LF instead of NUL.  This led to garbage at the
> end of the "extrakeys" setting that was written into setup.rc.  Fix
> this by replacing the final LF by NUL before calling set().

Disregard this for now.  I've just discovered some further issues 
involving "extrakeys", and I'd like to fix them all at once.

Ken

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH setup] Don't write garbage into the "extrakeys" user setting
  2017-11-27 21:36 [PATCH setup] Don't write garbage into the "extrakeys" user setting Ken Brown
  2017-11-27 22:20 ` Ken Brown
@ 2017-11-27 22:26 ` Jon Turney
  1 sibling, 0 replies; 6+ messages in thread
From: Jon Turney @ 2017-11-27 22:26 UTC (permalink / raw)
  To: cygwin-apps

On 27/11/2017 21:35, Ken Brown wrote:
> The ExtraKeysSetting destructor called UserSettings::set() on a string
> that was terminated by LF instead of NUL.  This led to garbage at the

What...?!?

> /*
>  * It stores them all in a contiguous memory buffer.  Each is one line of
>  * ASCII text terminated by LF.  THERE IS NO NUL-TERMINATION HERE, TAKE CARE!
>  */

Awesome... :)

> end of the "extrakeys" setting that was written into setup.rc.  Fix
> this by replacing the final LF by NUL before calling set().

Thanks for tracking this down.  Please apply.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH setup] Don't write garbage into the "extrakeys" user setting
  2017-11-27 22:20 ` Ken Brown
@ 2017-11-28  2:14   ` Ken Brown
  2017-11-28 12:58     ` Ken Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Ken Brown @ 2017-11-28  2:14 UTC (permalink / raw)
  To: cygwin-apps

On 11/27/2017 5:19 PM, Ken Brown wrote:
> On 11/27/2017 4:35 PM, Ken Brown wrote:
>> The ExtraKeysSetting destructor called UserSettings::set() on a string
>> that was terminated by LF instead of NUL.  This led to garbage at the
>> end of the "extrakeys" setting that was written into setup.rc.  Fix
>> this by replacing the final LF by NUL before calling set().
> 
> Disregard this for now.  I've just discovered some further issues 
> involving "extrakeys", and I'd like to fix them all at once.

Here's an amusing fact: "extrakeys" has apparently been useless for many 
years, so that the '-u' option to setup cannot have worked.  The reason 
is that '#' is a comment character in setup.rc, and the saved keys 
contain several of these.  (Originally the saved keys were kept in a 
file /etc/setup/last-extrakeys, which did not contain comments, but this 
was changed in 2009 to use /etc/setup/setup.rc.)

Here's an example from my own setup.rc after running 'setup -K ...':

extrakeys
	(public-key (dsa (p 
#00DE8D7944FA1731C7D66C17E3928E1CD6D06092B4DD549B96773B24F4F8C0609912EB2158379F7F37DF4BF3F42BF4131DAD1AC27B998398B635147B605F5D5276F0BBF66ECA4583A474A578C82A08EDE6802A74B39EAC62BC7EA7DE5BE721E23A51EDFC68B32A5F8462C071AFDCEA11576F39AC5646F87CEB37A67FE7A2421787#) 
(q #00C2D926EC5053000FF6BAFFA16D39161F8DE16BDF#) (g 
#0095E00E78F13C69274EE7724F2ECA34556ABA6B04EA05C06546F8CD7553807F22EF125AFE01541F82C2AF661C7F7036EDB3A0CC1F8BBB8E5E4F9CF01C179702415B9D2875EDD5AECACF85A87850E71DB1F3BA88EAE1C053ECC35507295C953E8DCBE701CE9BB3DB6E2AE35193A1503695338C448AB532571999BB4BFFFCBC9F2E#) 
(y 
#6F06E2269F93DC76794E63B6C9B56FAF0E57193FA890197BFB6C93BC9FE91C226EBACA0538CE229617E43A15972078574D907479375D4D3CCEA62C0A7A4E1F8D4D02401232DDF86471CD2D18F47E0C0AF9F32A36D0A609972DA79A762AFDBBD9B66F3AC26F490CB526CF9005B7FE4FDF560268EF4ED3E3ED7D9030BB6157D4B8#) 
) )

[The part below "extrakeys" is all one line.]

Everything after the first '#' is discarded, and "extrakeys" is 
effectively set to "(public-key (dsa (p ".

Out of sheer laziness, I'd like to fix this by no longer treating '#' as 
a comment character.  The only use of it I've seen is in site.cc to 
write a comment about dropped mirrors being saved, although there might 
be other uses that I've missed.

If this is not acceptable, then I guess we have to use something like 
'\#' to escape '#', or else choose a different comment character.

WDYT?

Ken

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH setup] Don't write garbage into the "extrakeys" user setting
  2017-11-28  2:14   ` Ken Brown
@ 2017-11-28 12:58     ` Ken Brown
  2017-11-28 13:59       ` Ken Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Ken Brown @ 2017-11-28 12:58 UTC (permalink / raw)
  To: cygwin-apps

On 11/27/2017 9:14 PM, Ken Brown wrote:
> On 11/27/2017 5:19 PM, Ken Brown wrote:
>> On 11/27/2017 4:35 PM, Ken Brown wrote:
>>> The ExtraKeysSetting destructor called UserSettings::set() on a string
>>> that was terminated by LF instead of NUL.  This led to garbage at the
>>> end of the "extrakeys" setting that was written into setup.rc.  Fix
>>> this by replacing the final LF by NUL before calling set().
>>
>> Disregard this for now.  I've just discovered some further issues 
>> involving "extrakeys", and I'd like to fix them all at once.
> 
> Here's an amusing fact: "extrakeys" has apparently been useless for many 
> years, so that the '-u' option to setup cannot have worked.  The reason 
> is that '#' is a comment character in setup.rc, and the saved keys 
> contain several of these.  (Originally the saved keys were kept in a 
> file /etc/setup/last-extrakeys, which did not contain comments, but this 
> was changed in 2009 to use /etc/setup/setup.rc.)
> 
> Here's an example from my own setup.rc after running 'setup -K ...':
> 
> extrakeys
>      (public-key (dsa (p 
> #00DE8D7944FA1731C7D66C17E3928E1CD6D06092B4DD549B96773B24F4F8C0609912EB2158379F7F37DF4BF3F42BF4131DAD1AC27B998398B635147B605F5D5276F0BBF66ECA4583A474A578C82A08EDE6802A74B39EAC62BC7EA7DE5BE721E23A51EDFC68B32A5F8462C071AFDCEA11576F39AC5646F87CEB37A67FE7A2421787#) 
> (q #00C2D926EC5053000FF6BAFFA16D39161F8DE16BDF#) (g 
> #0095E00E78F13C69274EE7724F2ECA34556ABA6B04EA05C06546F8CD7553807F22EF125AFE01541F82C2AF661C7F7036EDB3A0CC1F8BBB8E5E4F9CF01C179702415B9D2875EDD5AECACF85A87850E71DB1F3BA88EAE1C053ECC35507295C953E8DCBE701CE9BB3DB6E2AE35193A1503695338C448AB532571999BB4BFFFCBC9F2E#) 
> (y 
> #6F06E2269F93DC76794E63B6C9B56FAF0E57193FA890197BFB6C93BC9FE91C226EBACA0538CE229617E43A15972078574D907479375D4D3CCEA62C0A7A4E1F8D4D02401232DDF86471CD2D18F47E0C0AF9F32A36D0A609972DA79A762AFDBBD9B66F3AC26F490CB526CF9005B7FE4FDF560268EF4ED3E3ED7D9030BB6157D4B8#) 
> ) )
> 
> [The part below "extrakeys" is all one line.]
> 
> Everything after the first '#' is discarded, and "extrakeys" is 
> effectively set to "(public-key (dsa (p ".
> 
> Out of sheer laziness, I'd like to fix this by no longer treating '#' as 
> a comment character.  The only use of it I've seen is in site.cc to 
> write a comment about dropped mirrors being saved, although there might 
> be other uses that I've missed.
> 
> If this is not acceptable, then I guess we have to use something like 
> '\#' to escape '#', or else choose a different comment character.

Another possibility, which I think would require only minimal changes, 
is to treat '#' as a comment character only if it's the first 
non-whitespace character on a line.

Ken

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH setup] Don't write garbage into the "extrakeys" user setting
  2017-11-28 12:58     ` Ken Brown
@ 2017-11-28 13:59       ` Ken Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Ken Brown @ 2017-11-28 13:59 UTC (permalink / raw)
  To: cygwin-apps

On 11/28/2017 7:58 AM, Ken Brown wrote:
> On 11/27/2017 9:14 PM, Ken Brown wrote:
>> On 11/27/2017 5:19 PM, Ken Brown wrote:
>>> On 11/27/2017 4:35 PM, Ken Brown wrote:
>>>> The ExtraKeysSetting destructor called UserSettings::set() on a string
>>>> that was terminated by LF instead of NUL.  This led to garbage at the
>>>> end of the "extrakeys" setting that was written into setup.rc.  Fix
>>>> this by replacing the final LF by NUL before calling set().
>>>
>>> Disregard this for now.  I've just discovered some further issues 
>>> involving "extrakeys", and I'd like to fix them all at once.
>>
>> Here's an amusing fact: "extrakeys" has apparently been useless for 
>> many years, so that the '-u' option to setup cannot have worked.  The 
>> reason is that '#' is a comment character in setup.rc, and the saved 
>> keys contain several of these.  (Originally the saved keys were kept 
>> in a file /etc/setup/last-extrakeys, which did not contain comments, 
>> but this was changed in 2009 to use /etc/setup/setup.rc.)
>>
>> Here's an example from my own setup.rc after running 'setup -K ...':
>>
>> extrakeys
>>      (public-key (dsa (p 
>> #00DE8D7944FA1731C7D66C17E3928E1CD6D06092B4DD549B96773B24F4F8C0609912EB2158379F7F37DF4BF3F42BF4131DAD1AC27B998398B635147B605F5D5276F0BBF66ECA4583A474A578C82A08EDE6802A74B39EAC62BC7EA7DE5BE721E23A51EDFC68B32A5F8462C071AFDCEA11576F39AC5646F87CEB37A67FE7A2421787#) 
>> (q #00C2D926EC5053000FF6BAFFA16D39161F8DE16BDF#) (g 
>> #0095E00E78F13C69274EE7724F2ECA34556ABA6B04EA05C06546F8CD7553807F22EF125AFE01541F82C2AF661C7F7036EDB3A0CC1F8BBB8E5E4F9CF01C179702415B9D2875EDD5AECACF85A87850E71DB1F3BA88EAE1C053ECC35507295C953E8DCBE701CE9BB3DB6E2AE35193A1503695338C448AB532571999BB4BFFFCBC9F2E#) 
>> (y 
>> #6F06E2269F93DC76794E63B6C9B56FAF0E57193FA890197BFB6C93BC9FE91C226EBACA0538CE229617E43A15972078574D907479375D4D3CCEA62C0A7A4E1F8D4D02401232DDF86471CD2D18F47E0C0AF9F32A36D0A609972DA79A762AFDBBD9B66F3AC26F490CB526CF9005B7FE4FDF560268EF4ED3E3ED7D9030BB6157D4B8#) 
>> ) )
>>
>> [The part below "extrakeys" is all one line.]
>>
>> Everything after the first '#' is discarded, and "extrakeys" is 
>> effectively set to "(public-key (dsa (p ".
>>
>> Out of sheer laziness, I'd like to fix this by no longer treating '#' 
>> as a comment character.  The only use of it I've seen is in site.cc to 
>> write a comment about dropped mirrors being saved, although there 
>> might be other uses that I've missed.
>>
>> If this is not acceptable, then I guess we have to use something like 
>> '\#' to escape '#', or else choose a different comment character.
> 
> Another possibility, which I think would require only minimal changes, 
> is to treat '#' as a comment character only if it's the first 
> non-whitespace character on a line.

I've implemented this and will send a patch series shortly.

Ken

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-11-28 13:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-27 21:36 [PATCH setup] Don't write garbage into the "extrakeys" user setting Ken Brown
2017-11-27 22:20 ` Ken Brown
2017-11-28  2:14   ` Ken Brown
2017-11-28 12:58     ` Ken Brown
2017-11-28 13:59       ` Ken Brown
2017-11-27 22:26 ` Jon Turney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).