Review Request: [RFC] Make security widget validations work

Discussion:

Rajeesh K Nambiar

2011-07-31 12:36:14 UTC

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------

Review request for Network Management.

Summary
-------

WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.

I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!

Diffs
-----

libs/ui/security/wpapskwidget.h 7077836
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/nullsecuritywidget.h a0faa46
libs/ui/security/securityleap.h 5d9b034
libs/ui/security/securitywidget.h 71ebf4a
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34

Diff: http://git.reviewboard.kde.org/r/102157/diff

Testing
-------

Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.

Thanks,

Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110731/8fd1c15c/attachment.htm

Ilia Kats

2011-08-03 14:27:55 UTC

Permalink

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated July 31, 2011, 12:36 p.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/wpapskwidget.h 7077836
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/nullsecuritywidget.h a0faa46
libs/ui/security/securityleap.h 5d9b034
libs/ui/security/securitywidget.h 71ebf4a
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110803/20da0931/attachment.html>

Lamarque Vieira Souza

2011-08-06 17:03:40 UTC

Permalink

Post by Rajeesh K Nambiar

libs/ui/security/wepwidget.cpp, line 105
<http://git.reviewboard.kde.org/r/102157/diff/1/?file=30472#file30472line105>
That's not quite correct. A key can be either hex or ascii, while a passphrase can be anything, the actual key will be derived from the MD5-sum of the passphrase. So the original regex/validator was correct in that part. However, the regexes here specify ranges in length ({5,13} means the thing before that can be repeated from 5 to 13 times), while a hex key can be either 10 or 26, an ascii key either 5 or 13 characters. I think something like (asciiregex{5}|asciiregex{13}|hexregex{10}|hexregex{26}) will do what we want here. Also, the commented out old code should probably be removed completely.

I think NM includes a similar check in its code. Can you check if you implementation is compatible with NM's?

- Lamarque Vieira

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/#review5347
-----------------------------------------------------------

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated Aug. 6, 2011, 10:39 a.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110806/00adb7b1/attachment.html>

Lamarque Vieira Souza

2011-08-08 15:58:54 UTC

Permalink

Post by Lamarque Vieira Souza

I think NM includes a similar check in its code. Can you check if you implementation is compatible with NM's?

I have check and your tests are equals to NM's, so you just need to fix the 'valid(bool)' signal redefinition and this patch is good to go.

- Lamarque Vieira

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/#review5347
-----------------------------------------------------------

Post by Lamarque Vieira Souza
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated Aug. 7, 2011, 4:05 p.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110808/b9befdd0/attachment-0001.html>

Ilia Kats

2011-08-08 22:44:40 UTC

Permalink

Post by Lamarque Vieira Souza

I think NM includes a similar check in its code. Can you check if you implementation is compatible with NM's?
I have check and your tests are equals to NM's, so you just need to fix the 'valid(bool)' signal redefinition and this patch is good to go.

One more thing: the passphrase can consist of arbitrary characters, not just ascii. However, currently, the validator is set in the constructor, if it returns Invalid, characters like ?, ?, ? or % are not even accepted by the lineEdit. I think we need to move the call to setValidator into keyTypeChanged() and call it with 0 if passphrase is selected to remove the validator from the lineEdit.

- Ilia

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/#review5347
-----------------------------------------------------------

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110808/cff16cb0/attachment.html>

Rajeesh K Nambiar

2011-08-06 09:37:27 UTC

Permalink

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------

(Updated Aug. 6, 2011, 9:37 a.m.)

Review request for Network Management.

Changes
-------

Thanks for the review, Ilia.
You're right; WEP Hex key can have 10 or 26 digits, Ascii key can have 5 or 13 characters. WEP Passphrase should have 1 to 64 chars.

Summary
-------

WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.

I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!

Diffs (updated)
-----

libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpapskwidget.h 7077836

Diff: http://git.reviewboard.kde.org/r/102157/diff

Testing
-------

Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.

Thanks,

Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110806/7f305e3c/attachment.html>

Rajeesh K Nambiar

2011-08-06 10:39:38 UTC

Permalink

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------

(Updated Aug. 6, 2011, 10:39 a.m.)

Review request for Network Management.

Changes
-------

Grr...Revision 2 won't build. Fixed in Rev3.

Summary
-------

WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.

I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!

Diffs (updated)
-----

libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836

Diff: http://git.reviewboard.kde.org/r/102157/diff

Testing
-------

Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.

Thanks,

Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110806/94f02e51/attachment.html>

Lamarque Vieira Souza

2011-08-06 17:11:41 UTC

Permalink

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated Aug. 6, 2011, 10:39 a.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110806/8cb81c6c/attachment.html>

Lamarque Vieira Souza

2011-08-06 17:14:38 UTC

Permalink

Post by Rajeesh K Nambiar

libs/ui/security/wepwidget.cpp, line 57
<http://git.reviewboard.kde.org/r/102157/diff/3/?file=30889#file30889line57>
As far as I know {10,26} means 10 or 26 already.

Sorry, I have just checked {10,26} means from 10 to 26. Your change is correct. I must have confused it PERL, where {10-26} means 10 to 26 and {10,26} means 10 or 26.

- Lamarque Vieira

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/#review5444
-----------------------------------------------------------

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated Aug. 6, 2011, 10:39 a.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110806/7cf3f139/attachment-0001.html>

Rajeesh K Nambiar

2011-08-06 18:01:06 UTC

Permalink

Post by Rajeesh K Nambiar

libs/ui/security/wepwidget.cpp, line 122
<http://git.reviewboard.kde.org/r/102157/diff/3/?file=30889#file30889line122>
You could have changed WepWidget::validate() into a slot and used instead of creating a slot that just call WepWidget::validate().

validate() is member of superclass SecurityWidget, and returns bool; which is reimplemented by many subclasses, so it wasn't very appealing to change them all.
Is there a better way?

- Rajeesh

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/#review5444
-----------------------------------------------------------

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated Aug. 6, 2011, 10:39 a.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110806/69aaa196/attachment.html>

Lamarque Vieira Souza

2011-08-06 23:29:55 UTC

Permalink

Post by Rajeesh K Nambiar

validate() is member of superclass SecurityWidget, and returns bool; which is reimplemented by many subclasses, so it wasn't very appealing to change them all.
Is there a better way?

That is correct way in my point of view. I have just changed SecurityWidget::validate() into a slot it seems to be working, now you just need to use it instead of creating a new slot.

- Lamarque Vieira

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/#review5444
-----------------------------------------------------------

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated Aug. 6, 2011, 10:39 a.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110806/c820a40a/attachment.html>

Rajeesh K Nambiar

2011-08-07 07:10:54 UTC

Permalink

Post by Rajeesh K Nambiar

Hm, I can't seem to make it work. Emitting valid() from validate() gives compile error "In member function ?virtual bool WepWidget::validate() const?: error: passing ?const WepWidget? as ?this? argument of ?virtual void WepWidget::valid(bool)? discards qualifiers [-fpermissive]".

Furthermore, that would cause infinite loop as valid() is connected to WirelessSecuritySettingWidget::validate() which in turn calls SecurityWidget::validate() again.

- Rajeesh

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/#review5444
-----------------------------------------------------------

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated Aug. 6, 2011, 10:39 a.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110807/9c790480/attachment-0001.html>

Lamarque Vieira Souza

2011-08-07 14:01:56 UTC

Permalink

Post by Rajeesh K Nambiar

validate() is member of superclass SecurityWidget, and returns bool; which is reimplemented by many subclasses, so it wasn't very appealing to change them all.
Is there a better way?
That is correct way in my point of view. I have just changed SecurityWidget::validate() into a slot it seems to be working, now you just need to use it instead of creating a new slot.
Hm, I can't seem to make it work. Emitting valid() from validate() gives compile error "In member function ?virtual bool WepWidget::validate() const?: error: passing ?const WepWidget? as ?this? argument of ?virtual void WepWidget::valid(bool)? discards qualifiers [-fpermissive]".
Furthermore, that would cause infinite loop as valid() is connected to WirelessSecuritySettingWidget::validate() which in turn calls SecurityWidget::validate() again.

Ok, the infinite loop really happens. So I will revert my last commit and you can keep this part as it is.

- Lamarque Vieira

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/#review5444
-----------------------------------------------------------

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated Aug. 6, 2011, 10:39 a.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110807/b2014050/attachment-0001.html>

Lamarque Vieira Souza

2011-08-07 13:53:50 UTC

Permalink

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/#review5473
-----------------------------------------------------------

libs/ui/security/eapmethodstack.h
<http://git.reviewboard.kde.org/r/102157/#comment4917>

Qt's classes also inherit signals, since this one has been declared in SecurityWidget you do not need to declare it again.

libs/ui/security/leapauthwidget.h
<http://git.reviewboard.kde.org/r/102157/#comment4909>

Qt's classes also inherit signals, since this one has been declared in SecurityWidget you do not need to declare it again.

libs/ui/security/nullsecuritywidget.h
<http://git.reviewboard.kde.org/r/102157/#comment4911>

Use Q_SIGNALS to follow code style (below we use Q_SLOTS).

libs/ui/security/securityleap.h
<http://git.reviewboard.kde.org/r/102157/#comment4913>

Use Q_SIGNALS to follow code style (below we use Q_SLOTS).

libs/ui/security/securitywidget.h
<http://git.reviewboard.kde.org/r/102157/#comment4916>

I think "connected" is more used than "hooked" when talking about signals and slots.

libs/ui/security/securitywidget.h
<http://git.reviewboard.kde.org/r/102157/#comment4912>

Use Q_SIGNALS to follow code style (below we use Q_SLOTS).

libs/ui/security/wepauthwidget.h
<http://git.reviewboard.kde.org/r/102157/#comment4914>

Use Q_SIGNALS to follow code style (below we use Q_SLOTS).

libs/ui/security/wepwidget.h
<http://git.reviewboard.kde.org/r/102157/#comment4918>

Qt's classes also inherit signals, since this one has been declared in SecurityWidget you do not need to declare it again.

libs/ui/security/wirelesssecuritysettingwidget.cpp
<http://git.reviewboard.kde.org/r/102157/#comment4908>

"signal". Anyway, I think this comment is not necessary.

libs/ui/security/wpaauthwidget.h
<http://git.reviewboard.kde.org/r/102157/#comment4915>

Use Q_SIGNALS to follow code style (below we use Q_SLOTS).

- Lamarque Vieira

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated Aug. 6, 2011, 10:39 a.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110807/a9fc614f/attachment.html>

Rajeesh K Nambiar

2011-08-07 16:05:18 UTC

Permalink

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------

(Updated Aug. 7, 2011, 4:05 p.m.)

Review request for Network Management.

Changes
-------

Thanks for the review.
Need to use signals (or Q_SIGNALS, which used) in the subclasses also to avoid linking error.

Summary
-------

WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.

I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!

Diffs (updated)
-----

libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836

Diff: http://git.reviewboard.kde.org/r/102157/diff

Testing
-------

Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.

Thanks,

Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110807/790fe2a4/attachment.html>

Lamarque Vieira Souza

2011-08-08 15:38:06 UTC

Permalink

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated Aug. 7, 2011, 4:05 p.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110808/04f855b6/attachment.html>

Lamarque Vieira Souza

2011-08-08 15:51:03 UTC

Permalink

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated Aug. 7, 2011, 4:05 p.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/eapmethodstack.h 552f5c9
libs/ui/security/leapauthwidget.h 3c01e1d
libs/ui/security/nullsecuritywidget.h dc84fb5
libs/ui/security/securityleap.h 402a7af
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepauthwidget.h 6ff721b
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
libs/ui/security/wpaauthwidget.h 91e7495
libs/ui/security/wpapskwidget.h 7077836
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110808/a21aaa06/attachment.html>

Rajeesh K Nambiar

2011-08-09 13:04:31 UTC

Permalink

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------

(Updated Aug. 9, 2011, 1:04 p.m.)

Review request for Network Management.

Changes
-------

Simplified patch, thanks to Lamarque's review.
Also sets the validator in keyTypeChanged - this makes Passphrase to take any characters (tested with Unicode characters), thanks to Ilia.

Summary
-------

WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.

I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!

Diffs (updated)
-----

libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34

Diff: http://git.reviewboard.kde.org/r/102157/diff

Testing
-------

Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.

Thanks,

Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110809/8d969c16/attachment.html>

Commit Hook

2011-08-11 16:08:12 UTC

Permalink

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102157/#review5650
-----------------------------------------------------------

This review has been submitted with commit 92ecd626573a8b918ff50c04ef4ea6bcc272566e by Ilia Kats to branch nm09.

- Commit

Post by Rajeesh K Nambiar
-----------------------------------------------------------
http://git.reviewboard.kde.org/r/102157/
-----------------------------------------------------------
(Updated Aug. 9, 2011, 1:04 p.m.)
Review request for Network Management.
Summary
-------
WEP key validation (should have only 5 to 13 ASCII characters for Ascii key; and 10 to 26 Hex characters for Hex key) wasn't working. Most of the bits were present, here's a gross hack to make it really work. Only implemented and tested WEP validation, but can be extended for other security types as well.
I'm not sure this could be the best/easiest implementation, spent a night figuring out the right signal/slots :-). If this hack could be refined, please let me know!
Diffs
-----
libs/ui/security/securitywidget.h c536cf6
libs/ui/security/wepwidget.h e78338c
libs/ui/security/wepwidget.cpp f024555
libs/ui/security/wirelesssecuritysettingwidget.cpp fbc2c34
Diff: http://git.reviewboard.kde.org/r/102157/diff
Testing
-------
Tested with WEP Hex and ASCII key validations. "OK" button of connection editor widget is now properly enabled/disabled upon changing key.
Thanks,
Rajeesh

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-networkmanager/attachments/20110811/71413dbc/attachment.html>