[Edbrowse-dev] Disabling local echo for password fields

[Edbrowse-dev] Disabling local echo for password fields

[Dominique Martinet]

Hi all,


(I'm new here, just recently found out about edbrowse and I like the
concept)


This discussion started on github, I will write a short recap for people
not following github issues: https://github.com/CMB/edbrowse/pull/29


Basically, I'd like to disable local echo to keep passwords for
appearing in plain text, so people in the same room will not be able to
glance over my shoulder.
This is consistent with most unix login utilities (initial login, ssh
password prompt, etc)

One of the issue that was raised is that I only made the change for HTTP
auth, but that leaves many password input fields visible so it is a very
incomplete fix.


There are two sides to input fields:
 - the input itself, as things stand, small input fields have to be
entered as a full line e.g. i2=mypass, which cannot be easily hidden as
we read lines one at a time.
CMB suggested adding a new input function, for example 'pi' for 'private
input', that would prompt for the content of the input box and could be
more easily hidden.
If the extra command is a burden, we could make 'i' work again in browse
mode, and decide if there should be local echo based on the input field
type=password

 - the input content printed back out when you display the buffer
content, e.g. 'p' after entering.
This text could be starred out, either based on type=password, or if a
new command is implemented we could just always display stars whenever
that private input command is used (as the user likely would not want
the input of these commands printed)


What are others thoughts of adding such a command?

I like the idea and don't see much downsides, anyone wanting to use
i2=pass still can if they want to and it would be appropriate.
It might be slightly more confusing for new users but I think the
concept of no-echo is common enough in the unix world, I'm not too sure
about windows.



Other points that were addressed:
 - windows users will need a different way to disable echo, I'm not
familiar with windows terminal/input window at all, but I understand the
need. I will split the linux tcsetattr code in a subfunction that will
need a windows equivalent.

 - there is a second patch about adding CURLAUTH_NEGOTIATE back as an
option as I would use it, Adam Thompson disabled it back in 2014;
waiting for his or anyone's input if it is still useful to keep disabled
by default or not.


Thank you for reading,
-- 
Dominique Martinet | Asmadeus

[Edbrowse-dev] Disabling local echo for password fields

[Karl Dahlke]

[-- Attachment #1: Type: text/plain, Size: 6628 bytes --]

Sure - here are a few thoughts.

1. The concept is fine - the implementation is not trivial, technically, or in its user interface.

2. Since we have zero windows edbrowse users at the moment, I'm fine with saying the feature only works on unix / linux for now.
The Windows port is more a proof of concept.
Let's be honest, windows users aren't really interested in command line programs.
There are a couple other features that only work on unix, so ok.
But, in the interest of a windows implementation some day,
I'd prefer a function in stringfile.c that turns echo on and off, using termios of course, and maybe some day having a windows equivalent that is a stub for now.
You'll notice in that same file the getch() function which is an msdos function, and has a unix equivalent, to grab a single character no echo.
This is used in the mail client, for quick one letter commands to step through your mail; not used in other parts of edbrowse so you may not have run into it,
but that's kind of a model for tty io and portability.
Put your echo on and off function in there, under #ifdef windows etc.

3. Test for isInteractive in the echo on off function, and have them do nothing if not interactive.
That way we don't have to test for isInteractive every time we call the function; just call it at the appropriate times.
This flag is set for interactive use, and is done in a unix / windows portable way.

4. Finally, call this function, on and off, when grabbing the password for http 401 authentication - piece of cake.

5. The easiest, and perhaps the only practical way to print stars on top of a password field is to muck with the print line routine.
I can't really change the password text in the buffer, because I am constantly comparing these fields with their javascript counterparts.
In other words, if I put stars in the buffer, a syncup routine will eventually think you typed stars in the password field,
and update the javascript variable, and pretty soon you're sending stars to the website as your password.
So you can only muck with the print routine at buffers.c line 255.
When characters in that line belong to <input type=password> print out stars instead.
This is not easy to do right now.
Line 240 fetches the line with mode 1, which strips all the hidden tags out, just the printable version,
so by line 255 I have no idea what characters come from what html fields.
You would need some earlier software, fetch the line with mode -1,
that has all the codes in it, then step through and look for
InternalCodeChar number < password text >
then check tagList[number],
is its action TAGACT_INPUT and is its itype TAGACT_PASSWORD,
and if yes then overwrite the characters in the fetched line, the one that is fetched with mode 1, with stars.
You can do that because the line fetched with mode 1 is allocated, and you can muck with it.
The line fetched with mode -1 is raw from the buffer and you shouldn't change it.

6. That all seems doable except I treat PASSWORD just like TEXT.
In other words, itype has no indication of password, it just knows text.
This is suppose to be the major input type.
text, submit, reset, radio, dropdown list, etc.
I wanted to implement a minor input type, t->itype_minor,
that would hold the types in decorate.c line 339.
Thus we have some foundational changes to make first, right?
But these should be made anyways for other reasons, syntax checking on the input etc.
Input type email looks like an email address, number is a number, etc.
uchar itype_minor in eb.h line 528
set the itype_minor when the itype is set in decorate.c
Now ... if(itype_minor == INP_PASSWORD) then replace the characters in the field with stars.

7. Ok that was a lot of work, but none of it hard, just plod along plod plod plod.
We must realize what we have though.
If you write the buffer to a file, the password is plain text once again.
That's rare, but you need to know it would work that way,
and if you then print that file later on, your password is visible.
Perhaps less rare, the et command to edit the page as pure text.
Sometimes I do this if I just want to muck with the text and not interact with the links any more.
At that point the html tags are gone and the password shows up in plain text.

8. Now for input i2=password, which I have no way to anticipate until you have already typed it in with echo.
A private command was suggested, and I'm ok with that, but would probably call it private rather than pi,
cause it's used so rarely I don't mind it having more than 2 letters, and we're kinda running out of 2 letter commands,
and they're rather cryptic anyways.
Sometimes I'm tempted to turn a few more rarely used 2 letter commands into more meaningful words, but we have to be careful changing a preexisting user interface, no matter how clunky it is.
Maybe priv, or noecho, we can discuss the actual command later.
It would set a flag and disable echo.
buffers.c line 385 inputLine() would get the line as usual, but turn echo on after it got the line.
A one time silent read.
Question - how would this work in readline mode, when I'm calling readline() rather than fgets()?
I don't know anything about readline; never use it.

9. The noecho command should probably print "echo off" or some such so you know you're ok to enter your password.
This should be an international message of course.
There are a lot of old messages we don't use any more, and they have been replaced with 0, so use one of those slots before you make additional entries to the file.
Example lang/msg-en line 508
Put the new English message there, and MSG_EchoOff in messages.h line 608, and we use that, and my friends translate as time permits.
We do have some international users, so this is worth the bother.

10. I greatly appreciate any help you can give. This project should have 50 fulltime programmers; it has 4 part time volunteers, who all have busy lives.
Don't get discouraged if I redirect you in some way; we could really use your help.

11. Any of these changes should be documented in the users guide, and I'm happy to do that.
It's written in raw html and not many people want to muck with that.
This is of course the last step, but an important one; and also mention the new feature(s) in CHANGES.

12. I can't comment on the curl ssl authentication discussion, I'm not sure why the change was made, or if it was getting around a bug in an old version of curl, a bug that isn't there any more,
or what we should really do here; I'll leave that to Chris and Adam.

Cheers,

Karl Dahlke

Re: [Edbrowse-dev] Disabling local echo for password fields

[Chris Brannon]

Karl Dahlke <eklhad@comcast.net> writes:

> But, in the interest of a windows implementation some day,
> I'd prefer a function in stringfile.c that turns echo on and off,
*SNIP*
> 4. Finally, call this function, on and off, when grabbing the password for http 401 authentication - piece of cake.

Ok, so Dominique should resend the patch to disable echo with the
changes you outlined?  This is orthogonal to the private command for
entering form data, and the function to toggle echo could be used for
the private command as well, if / when it is implemented.

-- Chris

Re: [Edbrowse-dev] Disabling local echo for password fields

[Dominique Martinet]

Chris Brannon wrote on Fri, Jul 07, 2017:
> Karl Dahlke <eklhad@comcast.net> writes:
> 
> > But, in the interest of a windows implementation some day,
> > I'd prefer a function in stringfile.c that turns echo on and off,
> *SNIP*
> > 4. Finally, call this function, on and off, when grabbing the password for http 401 authentication - piece of cake.
> 
> Ok, so Dominique should resend the patch to disable echo with the
> changes you outlined?  This is orthogonal to the private command for
> entering form data, and the function to toggle echo could be used for
> the private command as well, if / when it is implemented.

Yes, I will split this in two patches:

 - first patch implement the echo on/off in a function and call it only
for http auth, feature-equivalent to this patch

 - second (new) patch will attempt to do the 'priv' command

I am afraid I do not have much more free time than anyone here, so will
try to do this over the weekend.

Thank you Karl for the very detailed answer, I should be able to wade
through something

-- 
Dominique

Re: [Edbrowse-dev] Disabling local echo for password fields

[Dominique Martinet]

Karl Dahlke wrote on Mon, Jul 03, 2017:
> 4. Finally, call this function, on and off, when grabbing the password
> for http 401 authentication - piece of cake.

Up to here was easy, done and pushed again on github
( https://github.com/CMB/edbrowse/pull/29 should probably update )

> 5. [...]
> Line 240 fetches the line with mode 1, which strips all the hidden tags out, just the printable version,
> so by line 255 I have no idea what characters come from what html fields.
> You would need some earlier software, fetch the line with mode -1,
> that has all the codes in it, then step through and look for
> InternalCodeChar number < password text >
> then check tagList[number],
> is its action TAGACT_INPUT and is its itype TAGACT_PASSWORD,
> and if yes then overwrite the characters in the fetched line, the one
> that is fetched with mode 1, with stars.

That seems rather annoying: you have to parse the line twice, then try
and match the first parse with the second parse to check every input
fields.

I'm looking at the code with naive eyes and I think removeHiddenNumbers
would be a good place for this: the function is called with both the raw
buffer and writes the new buffers only used for display (so we can muck
with the output on the fly), and does the checks for InternalCodeChar
already so we just have to handle '<' slightly differently from the
other codes if the tag is INP_PW

Now, removeHiddenNumbers is called multiple times at different places;
it might be best to have two variants of the function (or an option) and
in displayLine call fetchLine with 0 (copy the buffer but does not run
removeHiddenNumber) then call our variant instead.

I've pushed a first version that just hijacks the function as ugly proof
of concept, and because it takes care of 'et' and 'w' commands as well
without any extra code.

> 6. That all seems doable except I treat PASSWORD just like TEXT.

turns out INP_PW is already a 'major' itype, so this worked without this
part.

> 8. Now for input i2=password, which I have no way to anticipate until
> you have already typed it in with echo.
> [...]
> It would set a flag and disable echo.
> buffers.c line 385 inputLine() would get the line as usual, but turn
> echo on after it got the line.
> A one time silent read.
> Question - how would this work in readline mode, when I'm calling
> readline() rather than fgets()?
> I don't know anything about readline; never use it.

I'd rather not use readline/regular read function but call gets within
the same command, just like http auth handles its own reads, maybe using
the same prompt_and_read function.
readline will likely remember the password in its own history and might
mess with echo by itself (it's really not meant for password input)

> 9. The noecho command should probably print "echo off" or some such so
> you know you're ok to enter your password.
> This should be an international message of course.

A good place to discuss both the actual command and the message.
I'd use ipass and MSG_Password, as it likely will be used for passwords
(and I can be lazy and not add a new message)
I think 'priv' or 'private' is not obvious enough that it talks about
input, hence the leading 'i'
The function could be used for non-password fields and set itype to
INP_PW on the fly if it is not already.

> 11. Any of these changes should be documented in the users guide, and
> I'm happy to do that.
> It's written in raw html and not many people want to muck with that.
> This is of course the last step, but an important one; and also
> mention the new feature(s) in CHANGES.

I actually do not mind editing html, will do once we all agree on the
interface details.


> 12. I can't comment on the curl ssl authentication discussion, I'm not
> sure why the change was made, or if it was getting around a bug in an
> old version of curl, a bug that isn't there any more,
> or what we should really do here; I'll leave that to Chris and Adam.

Still waiting on this bit; if the old change (making libcurl not use
NEGOTIATE) is no longer needed it would certainly be easier to just
enable it back.
I'm happy with an option though, and will update it to reuse unused
values for messages instead of adding new ones if we stay with that.

-- 
Dominique

[Edbrowse-dev] Disabling local echo for password fields

[Karl Dahlke]

I will have to defer to Chris for most of this.
I am off in another branch, the duktape branch.
Literally up to my elbows in the conversion work.
If Chris thinks the patches look ok he can integrate them into master branch.

You're right, mucking with removeHiddenNumbers() is probably the right way to do it,
and I can't think of any problems here, but I'd have to check all the places it is called and see if there
are any issues with stars instead of text, I don't think there are.
And it has the advantage that you can write the buffer to a file etc and it still has the stars.
Good call.

Yes I forgot Password was a major type, honestly it probably shouldn't be.
It's a minor variation on the text type, but it is what it is, so carry on.

Karl Dahlke

[Edbrowse-dev] Disabling local echo for password fields

[Karl Dahlke]

[-- Attachment #1: Type: text/plain, Size: 2160 bytes --]

I got the patch out of github, it will take a while to look through, not a lot of code but a lot of little changes all over the place.
Here is the ipass block with some thoughts.

 /* special command for hidden input */
 if (!strncmp(line, "ipass", 5)) {
 char *p, *c;
 char buffer[MAXUSERPASS];
 int realtotal;
 if (!cw->browseMode && (cmd == 'i' || cx)) {
#  why query cmd and cx, neither has been set to anything at this point.
#  cmd is the default p for print, I think.
 setError(MSG_NoBrowse);
 return false;
 }
 if (endRange > startRange && cmd == 'i') {
#  again, cmd will not be set to i.
#  Did you test all these cases?  1,3ipass will probably not fall into this
#  block the way you want it to. You should test every pathway.
 setError(MSG_RangeI, c);
#  c has not been set.
# setError(MSG_RangeI, '=');
 return false;
 }

 s = line + 5;
#  is cx set to 0 at this point? I think so.
 if (isdigitByte(*s))
 cx = strtol(s, (char **)&s, 10);
 else if (*s == '$')
 cx = -1, ++s;
 /* XXX try to guess cx if only one password input field? */

 cw->dot = endRange;
 p = (char*)fetchLine(cw->dot, -1);
 findInputField(p, 1, cx, &n, &realtotal, &tagno);
 debugPrint(5, "findField returns %d.%d", n, tagno);
 if (!tagno) {
 fieldNumProblem(0, "ipass", cx, n, realtotal);
 return false;
 }

 prompt_and_read(MSG_Password, buffer, MAXUSERPASS,
 MSG_PasswordLong, true);

 tagList[tagno]->itype = INP_PW;
#  Hold it! I have a real problem overriding the html tag type.
#  Mostly on philosophical grounds. I think others will as well.
#  Chris says it is always password when it's suppose to be a password,
#  almost always, so don't think we should change it.
#  In an extreme case it could have been a select list, menu of choices,
#  and now it's just a password text field and I'm sure that will
#  make something blow up somewhere.

 rc = infReplace(tagno, buffer, true);
 return rc;
 }

I haven't looked at the other routines.
edbrowse is a fragile thing, maybe because it isn't coded well,
I admit that, it just means we have to look at each line of code as it walks in the door.

Karl Dahlke

Re: [Edbrowse-dev] Disabling local echo for password fields

[Dominique Martinet]

Karl Dahlke wrote on Sun, Jul 09, 2017:
> I got the patch out of github, it will take a while to look through,
> not a lot of code but a lot of little changes all over the place.

Thank you; I was not sure on where to add code first so the code itself
was done quickly/is not polished.
If the place/global logic fits then it is a good start for me, I should
have been more explicit about that.

>  if (!cw->browseMode && (cmd == 'i' || cx)) {
> #  why query cmd and cx, neither has been set to anything at this point.
> #  cmd is the default p for print, I think.

Eep, you've caught some shameful copy/paste error here as I copied that
from the 'i' command block.
I had meant to only test browseMode (same for range)

>  setError(MSG_NoBrowse);
>  return false;
>  }
>  if (endRange > startRange && cmd == 'i') {
> #  again, cmd will not be set to i.
> #  Did you test all these cases?  1,3ipass will probably not fall into this
> #  block the way you want it to. You should test every pathway.
>  setError(MSG_RangeI, c);
> #  c has not been set.
> # setError(MSG_RangeI, '=');

MSG_RangeI is 'i' specific, we're going to need a new message (or make
the msg take a string as argument like fieldInputField)


> #  is cx set to 0 at this point? I think so.

yes


>  tagList[tagno]->itype = INP_PW;
> #  Hold it! I have a real problem overriding the html tag type.
> #  Mostly on philosophical grounds. I think others will as well.
> #  Chris says it is always password when it's suppose to be a password,
> #  almost always, so don't think we should change it.
> #  In an extreme case it could have been a select list, menu of choices,
> #  and now it's just a password text field and I'm sure that will
> #  make something blow up somewhere.

Right, I hadn't thought of radio and other type of fields as there
weren't any on the page I tested.
I agree something will likely break in that case, but I would like to
disable echo for informations entered with that even if the field is not
set -- I think in that case it makes more sense to revert INP_PW as a
minor type, I will implement that as you suggested in your earlier mail.

-- 
Dominique

Re: [Edbrowse-dev] Disabling local echo for password fields

[Chris Brannon]

Dominique Martinet <asmadeus@codewreck.org> writes:

> Right, I hadn't thought of radio and other type of fields as there
> weren't any on the page I tested.
> I agree something will likely break in that case, but I would like to
> disable echo for informations entered with that even if the field is not
> set -- I think in that case it makes more sense to revert INP_PW as a
> minor type, I will implement that as you suggested in your earlier mail.

The inp_types list in decorate.c is also duplicated in html.c, and you
missed that one in the patch to add the new minor type.
Couldn't we just get rid of the inp_types from html.c and make inp_types
from decorate.c global?
Maybe some of these constant strings belong in their own file, like
html-constants.c or whatever, but that's a discussion for another day.

I'd also say that the function infShow needs to be reworked a little, so
that it shows the minor type, and not just text.
I'd like Karl's opinion on that.  If he agrees, I wouldn't mind doing
that work after your patches are submitted.

Aside from all of that, your patchset looks pretty good.
I still want Adam's opinion on the auth negotiation.  I don't know if
his issue still exists, or whether it was subsequently fixed by libcurl
/ other software.

Regards,
-- Chris

Re: [Edbrowse-dev] Disabling local echo for password fields

[Dominique Martinet]

Chris Brannon wrote on Mon, Jul 10, 2017:
> The inp_types list in decorate.c is also duplicated in html.c, and you
> missed that one in the patch to add the new minor type.
> Couldn't we just get rid of the inp_types from html.c and make inp_types
> from decorate.c global?

I have done just that: removed inp_types in html.c and declared as
extern in eb.h
I think 'number' might also need to be moved to minor, but did not take
the time to change that.

> I'd also say that the function infShow needs to be reworked a little, so
> that it shows the minor type, and not just text.
> I'd like Karl's opinion on that.  If he agrees, I wouldn't mind doing
> that work after your patches are submitted.

That would make sense to me, I have made both inp_types and inp_others
global at the same time so you will be able to use it.

> Aside from all of that, your patchset looks pretty good.
> I still want Adam's opinion on the auth negotiation.  I don't know if
> his issue still exists, or whether it was subsequently fixed by libcurl
> / other software.

I have repushed the series without the last curl auth negotiation patch,
so we are not held by that. The patch is still available in my master
branch on github ( https://github.com/martinetd/edbrowse )

I have also added a first draft to the userguide, both in English and
French, for the new command.
We cannot change the command name once this is merged so now is a good
time to complain about my naming sense!


-- 
Dominique

Re: [Edbrowse-dev] Disabling local echo for password fields

[Chris Brannon]

Dominique Martinet <asmadeus@codewreck.org> writes:

> I have done just that: removed inp_types in html.c and declared as
> extern in eb.h
> I think 'number' might also need to be moved to minor, but did not take
> the time to change that.

I think the itype_minor idea is a good one, because all these minor
types are just text for the purposes of edbrowse.  We can add new
ones if we ever need to do that; they'll still be handled as text.
The thing I'm a little concerned about is the loss of information.
I didn't really think this through when I read your patches yesterday.
When ipass is used successfully to edit a form field, its itype_minor
automatically becomes INP_PW.  That loses the old itype_minor.  There
may be cases where we need it, E.G., for the DOM.
For instance, what if some JS code needs to find the fields having type
"email"?  This is a bit contrived.
Perhaps we should add a "maskedText" flag to struct htmlTag, set that in
ipass, and use it to determine whether or not to print the stars for
masked fields?  Or maybe my concern is overblown, I don't know.

Sorry for being a harsh reviewer.  I really do like your work,
and I want to see it merged.

-- Chris

Re: [Edbrowse-dev] Disabling local echo for password fields

[Dominique Martinet]

Chris Brannon wrote on Wed, Jul 12, 2017:
> I think the itype_minor idea is a good one, because all these minor
> types are just text for the purposes of edbrowse.  We can add new
> ones if we ever need to do that; they'll still be handled as text.
> The thing I'm a little concerned about is the loss of information.
> I didn't really think this through when I read your patches yesterday.
> When ipass is used successfully to edit a form field, its itype_minor
> automatically becomes INP_PW.  That loses the old itype_minor.  There
> may be cases where we need it, E.G., for the DOM.
> For instance, what if some JS code needs to find the fields having type
> "email"?  This is a bit contrived.

Right, I do not think code ought to rely on that (as they should use the
field id) but that does not mean such does not exist, we probably should
preserve this when needed.
On the other hand, I do not think people would use 'ipass' to enter
their email address, but I want to see the output masked when someones
uses ipass just in case some website "forgot" to set the password type.


What occured to me when writing the user guide part is that there is no
going back from this either, once a field has been set as password you
can still manipulate it with substitutions but you will not ever be able
to see the result.
There is no "unmask" or "print anyway" command that would do that.

I am not sure how much this would be useful in practice, though, so it
is probably OK to put this on some "maybe later" list...


> Perhaps we should add a "maskedText" flag to struct htmlTag, set that in
> ipass, and use it to determine whether or not to print the stars for
> masked fields?  Or maybe my concern is overblown, I don't know.

It is probably cleaner to keep it separate and having an extra bit
should not hurt much, I will go for that and submit a new revision
tonight or tomorrow.


> Sorry for being a harsh reviewer.  I really do like your work,
> and I want to see it merged.

No worry, I am used to much worse and this is better than accepting new
bugs too easily :)

-- 
Dominique

Re: [Edbrowse-dev] Disabling local echo for password fields

[Chris Brannon]

Dominique Martinet <asmadeus@codewreck.org> writes:

> There is no "unmask" or "print anyway" command that would do that.

Yes, we can always add an unmask command if we really need it, and it'd
just clear the bit.

I also noticed that when parsing input elements that have type="text",
itype_minor is never set.  So for <input type="text"> it's always 0 (which
corresponds to INP_DATE).  I'd say we need a "default" placeholder at
the end of the minor types enum, to specify that this element is plain
old "text", and htmlInputHelper() from decorate.c should look something
like this:

void htmlInputHelper(struct htmlTag *t)
{
	int n = INP_TEXT;
	int len;
	char *myname = (t->name ? t->name : t->id);
	const char *s = attribVal(t, "type");
	if (stringEqual(t->info->name, "button")) {
		n = INP_BUTTON;
	} else if (s) {
		n = stringInListCI(inp_types, s);
		if (n == INP_TEXT) {
			t->itype_minor = INP_DEFAULTMINOR;
		}

Also, when the type was not found in inp_types nor inp_others,
itype_minor should probably be set to INP_DEFAULTMINOR (or whatever you
care to call it).

-- Chris

Re: [Edbrowse-dev] Disabling local echo for password fields

[Dominique Martinet]

Chris Brannon wrote on Wed, Jul 12, 2017:
> Also, when the type was not found in inp_types nor inp_others,
> itype_minor should probably be set to INP_DEFAULTMINOR (or whatever you
> care to call it).

I'd say just keep 0 (as the field is init to 0) for that, so it both
does not matter if text does not set it and will be set "correctly" for
other fields as well.

Will anything care if I shuffle all the fields to the right for that, as
long as the text mapping and enum match?

-- 
Dominique

Re: [Edbrowse-dev] Disabling local echo for password fields

[Chris Brannon]

Dominique Martinet <asmadeus@codewreck.org> writes:

> Will anything care if I shuffle all the fields to the right for that, as
> long as the text mapping and enum match?

As long as they match, it should be fine I think.

-- Chris

[Edbrowse-dev] Disabling local echo for password fields

[Karl Dahlke]

I always put my default symbols at the beginning, so they correspond to zero, so they're just there from the start.
Example ebjs.h line 19.
I would prefer this approach.

Karl Dahlke

[Edbrowse-dev] Disabling local echo for password fields

[Karl Dahlke]

Yes, number is another minor type of text.
If you're making password a minor text type then number should be as well, if you don't mind and if you have the time.
Later on we can use this minor type to constrain the input, as other browsers do.
number has an obvious syntax, so does email, etc.

sure, infshow() should print text(number) or some such.

I continue my concern about overwriting or changing what html has declared, and Chris shares this concern.
This is just philosophy for now, can't think of a real-world issue, it just makes me queasy.
The mask bit would get around this.

bool masked:1;

Put this in with all the other boolean members in struct htmlTag.
Every struct I allocate is set to zero so you don't have to worry about that.
Add a new member and it is automatically false from the start.
The ipass command could set it to true, and then you could test

if(t->masked || t->inp_minor == INP_PW) { put in stars }

> We cannot change the command name once this is merged so now is a good

Right.
ipass or ipass2 or ipass$ it's not my favorite convention but truly I can't think of anything better.
It's so rarely used, and I'd never use it at all (personally), so in that sense I don't care.
Well I think it's fine if others agree.

Karl Dahlke

Re: [Edbrowse-dev] Disabling local echo for password fields

[Dominique Martinet]

Karl Dahlke wrote on Wed, Jul 12, 2017:
> The mask bit would get around this.
> 
> bool masked:1;

I have done that, you can find a new version in the pull request:
https://github.com/CMB/edbrowse/pull/29

> if(t->masked || t->inp_minor == INP_PW) { put in stars }

I preferred setting the masked flag when we set minor INP_PW instead

Since the masking/ipass commands no longer use the itype_minor field,
I have pushed it off separately, you can find it here:
https://github.com/martinetd/edbrowse/tree/itype_minor

This includes moving 'number' onto inp_others and displays it in
parenthesis in infShow, so a number would look like this with i?
"text (number) num_field_id"


(I have also left the curl auth patch in my master branch, on top of it
again)

I am still not convinced about github pull request system, but once this
gets in I will submit another PR for itype_minor if that is fine with
you (and eventually a third for the curl auth patch, if we can hear back
from Adam)

-- 
Dominique

Re: [Edbrowse-dev] Disabling local echo for password fields

[Chris Brannon]

Your pull request 29 looks good to me.
If Karl has no objections, it'll get pushed, and then we'll see about
the itype work.

-- Chris

[Edbrowse-dev] Disabling local echo for password fields

[Karl Dahlke]

[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]

> Your pull request 29 looks good to me.

yes, Chris, go ahead with it, and exercise it, and we can add tweaks and fixes later as needed.
I'll try to switch back from my duktape branch long enough to test it out,
but I have a lot of work to do where I am, a lot of mysteries to solve.

I optimistically put ipass and stars in my Changes file, on my branch, for when things get merged, I didn't change anything in userdoc, assuming you are doing that.
I call the next version 3.7.0, as I think duktape warrants a jump from 6 to 7.

Note on the curl method, if / when we put that in, if it is command switchable, the switch should include notifying the downstream js process,
since it can also fetch http files.
We do this already for other config things, like
vs toggle verify ssl connections.
Don't worry bout it for now, just something to keep in the back of your mind.
Let's make sure the stars and ipass are working first.

Geoff has duktape working on windows, for all of jsrt, except for one problem with the Date class, that we haven't figured out.
So that's pretty good.

Karl Dahlke

Re: [Edbrowse-dev] Disabling local echo for password fields

[Chris Brannon]

Dominique Martinet <asmadeus@codewreck.org> writes:

> Since the masking/ipass commands no longer use the itype_minor field,
> I have pushed it off separately, you can find it here:
> https://github.com/martinetd/edbrowse/tree/itype_minor

I grabbed that patch, and it looks good to me, so I will push it.  No
need to open a second PR.

-- Chris

Re: [Edbrowse-dev] Disabling local echo for password fields

[Chris Brannon]

Dominique Martinet <asmadeus@codewreck.org> writes:

> (I have also left the curl auth patch in my master branch, on top of it
> again)

Your curl auth patch is merged.  Sorry for the long wait; I really
wanted to get some clarification on that, and now I have it.

Thank you for all of these contributions!

-- Chris

Re: [Edbrowse-dev] Disabling local echo for password fields

[Dominique Martinet]

Chris Brannon wrote on Mon, Jul 17, 2017:
> Your curl auth patch is merged.  Sorry for the long wait; I really
> wanted to get some clarification on that, and now I have it.

Don't worry about it.
I did not see any clarification anywhere, did you get it in private?

(I'm mainly curious if the on/off flag turned out to be really required
for some users)


> Thank you for all of these contributions!

Thank you as well for the reviews and guidance :)

-- 
Dominique

Re: [Edbrowse-dev] Disabling local echo for password fields

[Chris Brannon]

Dominique Martinet <asmadeus@codewreck.org> writes:

> Don't worry about it.
> I did not see any clarification anywhere, did you get it in private?

Yes.  Basically MS NTLM breaks auth negotiation.  Since the problem
still persists, having this toggle is probably a good idea.

-- Chris