[GitHub] commons-lang issue #334: Code refactoring and cleaning

2018-06-23 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/334
  
Thanks @PascalSchumacher for confirming it's actually backward compatible 
(#TodayILearned - and hopefully will remember it).

@ingvarc , I agree on @aaabramov regarding the `+1`. When I read it, I 
first thought that was a `+= 1`, and then re-read it and understood his 
concern. Might be worth simplifying it. If you'd like to keep the commits 
simpler (which is great initiative, thanks!) you could try rebasing commits and 
push+force to your branch.

Good points @stokito. Not sure about branch prediction after the spectre 
bug. I think one of the mitigations involved disabling branch prediction. 
Though I think micro-benchmarking isn't priority in lang.

>Here developer sees a "happy path" first and only then he or she sees 
exception situation handling.
This is more natural to understand. As I remember this style was mentioned 
in Complete Code book.

Code Complete is one of my favourite books. For me, that code could be:

```java
private static void addProcessor(final String key, final Processor 
processor) {
if (ARCH_TO_PROCESSOR.containsKey(key)) {
throw new IllegalStateException("Key " + key + " already exists 
in processor map");
}
ARCH_TO_PROCESSOR.put(key, processor);
}
```

I think maybe developers would read the first `if` as a validation block. 
Then the normal behaviour, without the else (I think omitting the else in cases 
like this is also in Code Complete... or Pragmatic Programmer? Can't recall).

I think the other changes like removing class name, inverting some `if` 
conditions, and removing the exceptions not thrown look good, and the code 
readability keeps OK. Great contribution @ingvarc , thanks!



---


[GitHub] commons-lang issue #318: clean code

2018-06-23 Thread kinow
Github user kinow commented on the issue:

https://github.com/apache/commons-lang/pull/318
  
@JiangYongKang, thanks for your contribution.

Even though the behaviour is the same, in my opinion, the other code is 
easier to read, and fewer code doesn't necessarily mean better or more clean 
code.

Checking if a number if even or odd, for example, can be done with bit 
operation, or with a `if/else`. While the former is shorter, not all developers 
immediately understand it, nor know how to maintain/change it.


---


[GitHub] commons-lang issue #318: clean code

2018-06-23 Thread JiangYongKang
Github user JiangYongKang commented on the issue:

https://github.com/apache/commons-lang/pull/318
  
@stokito thinks


---


[jira] [Commented] (LANG-967) Code should never catch and ignore all Throwables

2018-06-23 Thread Sergey Ponomarev (JIRA)


[ 
https://issues.apache.org/jira/browse/LANG-967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16521312#comment-16521312
 ] 

Sergey Ponomarev commented on LANG-967:
---

As it was mentioned you shouldn't catch Throwable until you understood what are 
you doing and even you you need then catch Error explicitly and even better to 
catch specific error.
For example if you received OutOfMemoryError then it can be so low memory that 
you even can't write to logs or wrap OOM into another exception. But you can 
try to recover an application by clearing some in-memory caches or temporary 
stop processing. See Apache Derby 
[LowMemory|http://people.apache.org/~kristwaa/jacoco/org.apache.derby.iapi.services.memory/LowMemory.java.html]
 class and it's usage as example of recovery.

The mentioned Tomcat's ExceptionUtils.handleThrowable() looks quite weird. It 
rethrowth VirtualMachineError and ThreadDeath while all others are silently 
eaten. Maybe Tomcat's developers know what they are doing but this is 
definitely very custom logic.
I think we have nothing to do here and the ticket can be closed

> Code should never catch and ignore all Throwables
> -
>
> Key: LANG-967
> URL: https://issues.apache.org/jira/browse/LANG-967
> Project: Commons Lang
>  Issue Type: New Feature
>  Components: lang.exception.*
>Reporter: Sebb
>Priority: Major
> Fix For: Patch Needed
>
>
> Code should never catch and ignore all Throwables - ThreadDeath and 
> VirtualMachineError must be rethrown.
> It would be useful to provide a method to enforce this behaviour.
> For example,. as is done by the Tomcat method \[1] with source here \[2]
> Sample usage:
> {code}
> try {
> reader.close();
> } catch (Throwable u) {
> ExceptionUtils.handleThrowable(u);
> }
> {code}
> \[1] org.apache.tomcat.util.ExceptionUtils#handleThrowable()
> \[2] 
> https://svn.apache.org/repos/asf/tomcat/trunk/java/org/apache/tomcat/util/ExceptionUtils.java



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (LANG-1400) StringUtils: Add method for masking strings

2018-06-23 Thread Sergey Ponomarev (JIRA)


 [ 
https://issues.apache.org/jira/browse/LANG-1400?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Sergey Ponomarev updated LANG-1400:
---
Description: 
It would be great to have a function which allows to mask the original str by 
replacing it's
characters with the specified character-mask. For eaxmple to mask credit card:
{code}
mask("3566002020360505", 4, 4, '*') = "35660505"
{code}
Thus the number was hidden by asterisks while first and last four digits are 
unmasked and seen.

Common use case is to hide sensitive information from logs, by using it in 
toString() of classes or in inputs to log calls.

I think this is "must have" functionality for Commons Lang library because from 
my experience in almost all bit projects what I saw was their home grown 
masking function.
I think this is very important also because this is required for masking credit 
card numbers by PCI compliance. Also new GDPR rules requires to hide personal 
info as much as possible so masking of First and Last names now is required by 
laws.
To make the world safer place the utility for masking should be already 
existing in platform to avoid situations when developer think  "this is better 
to mask but I don't have enough time now so let's do this latter" until leak 
happens.
IMHO this should be implemented even in `String` class itself.

>From my experience what I saw was usually few masking usages and styles:
1. masking of passwords and names: only first and last symbols are shown, mask 
char is `*`
2. masking of credit cards: only first and last 4 or 6 symbols are shown, mask 
char is `*`.
3. credit card number shortest masking of last symbols i.e. 
`mask("4242424242424242") == " *4242"` but it's not so often used, I just 
wanted to mention.
4. not masking but showing a length just so see that value was passed. This can 
be easily achieved by usual `String.legth()` method.

There is already some pull request 
[https://github.com/apache/commons-lang/pull/332] but I decided to create the 
ticket because we also need to support masking for ToStringBuilder and I would 
like to propose [PR with my own implementation of mask() 
function|https://github.com/apache/commons-lang/pull/335].
If you accept my PR then I'll send another one with a new annotation 
@ToStringMasked in accordance to 
[@ToStringExclude|https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/builder/ToStringExclude.html]
 which will mark that fields should be masked. Possible usage I see like:
{code}
@ToStringMasked(unmaskedStart = 4, unmaskedEnd = 4) 
String creditCardNumber;

@ToStringMasked(unmaskedStart = 1, unmaskedEnd = 1) 
String password;
{code}

 

  was:
It would be great to have a function which allows to mask the original str by 
replacing it's
characters with the specified character-mask. For eaxmple to mask credit card:
{code}
mask("3566002020360505", 4, 4, '*') = "35660505"
{code}
Thus the number was hidden by asterisks while first and last four digits are 
unmasked and seen.

Common use case is to hide sensitive information from logs, by using it in 
toString() of classes or in inputs to log calls.

I think this is "must have" functionality for Commons Lang library because from 
my experience in almost all bit projects what I saw was their home grown 
masking function.
I think this is very important also because this is required for masking credit 
card numbers by PCI compliance. Also new GDPR rules requires to hide personal 
info as much as possible so masking of First and Last names now is required by 
laws.
To make the world safer place the utility for masking should be already 
existing in platform to avoid situations when developer think  "this is better 
to mask but I don't have enough time now so let's do this latter" until leak 
happens.
IMHO this should be implemented even in `String` class itself.

>From my experience what I saw was usually few masking usages and styles:
1. masking of passwords and names: only first and last symbols are shown, mask 
char is `*`
2. masking of credit cards: only first and last 4 or 6 symbols are shown, mask 
char is `*`.
3. credit card number shortest masking of last symbols i.e. 
`mask("4242424242424242") == " *4242"` but it's not so often used, I just 
wanted to mention.
4. not masking but showing a length just so see that value was passed. This can 
be easily achieved by usual `String.legth()` method.

There is already some pull request 
[https://github.com/apache/commons-lang/pull/332] but I decided to create the 
ticket because we also need to support masking for ToStringBuilder and I would 
like to propose [PR with my own implementation of mask() 
function|https://github.com/apache/commons-lang/pull/335].
If you accept my PR then I'll send another one with a new annotation 
@ToStringMasked in accordance to 
[@ToStringExclude|https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/l

[jira] [Updated] (LANG-1400) StringUtils: Add method for masking strings

2018-06-23 Thread Sergey Ponomarev (JIRA)


 [ 
https://issues.apache.org/jira/browse/LANG-1400?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Sergey Ponomarev updated LANG-1400:
---
Description: 
It would be great to have a function which allows to mask the original str by 
replacing it's
characters with the specified character-mask. For eaxmple to mask credit card:
{code}
mask("3566002020360505", 4, 4, '*') = "35660505"
{code}
Thus the number was hidden by asterisks while first and last four digits are 
unmasked and seen.

Common use case is to hide sensitive information from logs, by using it in 
toString() of classes or in inputs to log calls.

I think this is "must have" functionality for Commons Lang library because from 
my experience in almost all bit projects what I saw was their home grown 
masking function.
I think this is very important also because this is required for masking credit 
card numbers by PCI compliance. Also new GDPR rules requires to hide personal 
info as much as possible so masking of First and Last names now is required by 
laws.
To make the world safer place the utility for masking should be already 
existing in platform to avoid situations when developer think  "this is better 
to mask but I don't have enough time now so let's do this latter" until leak 
happens.
IMHO this should be implemented even in `String` class itself.

>From my experience what I saw was usually few masking usages and styles:
1. masking of passwords and names: only first and last symbols are shown, mask 
char is `*`
2. masking of credit cards: only first and last 4 or 6 symbols are shown, mask 
char is `*`.
3. credit card number shortest masking of last symbols i.e. 
`mask("4242424242424242") == " *4242"` but it's not so often used, I just 
wanted to mention.
4. not masking but showing a length just so see that value was passed. This can 
be easily achieved by usual `String.legth()` method.

There is already some pull request 
[https://github.com/apache/commons-lang/pull/332] but I decided to create the 
ticket because we also need to support masking for ToStringBuilder and I would 
like to propose [PR with my own implementation of mask() 
function|https://github.com/apache/commons-lang/pull/335].
If you accept my PR then I'll send another one with a new annotation 
@ToStringMasked in accordance to 
[@ToStringExclude|https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/builder/ToStringExclude.html]
 which will mark that fields should be masked. Possible usage I see like:
{code}
@ToStringMasked(unmaskedStart = 4, unmaskedEnd = 4) 
String creditCardNumber;
{code}

 

  was:
It would be great to have a function which allows to mask the original str by 
replacing it's
characters with the specified character-mask. For eaxmple to mask credit card:
{code}
mask("3566002020360505", 4, 4, '*') = "35660505"
{code}
Thus the number was hidden by asterisks while first and last four digits are 
unmasked and seen.

Common use case is to hide sensitive information from logs, by using it in 
toString() of classes or in inputs to log calls.

I think this is "must have" functionality for Commons Lang library because from 
my experience in almost all bit projects what I saw was their home grown 
masking function.
I think this is very important also because this is required for masking credit 
card numbers by PCI compliance. Also new GDPR rules requires to hide personal 
info as much as possible so masking of First and Last names now is required by 
laws.
To make the world safer place the utility for masking should be already 
existing in platform to avoid situations when developer think  "this is better 
to mask but I don't have enough time now so let's do this latter" until leak 
happens.
IMHO this should be implemented even in `String` class itself.

>From my experience what I saw was usually few masking usages and styles:
1. masking of passwords and names: only first and last symbols are shown, mask 
char is `*`
2. masking of credit cards: only first and last 4 or 6 symbols are shown, mask 
char is `*`.
3. credit card number shortest masking of last symbols i.e. 
`mask("4242424242424242") == " *4242"` but it's not so often used, I just 
wanted to mention.
4. not masking but showing a length just so see that value was passed. This can 
be easily achieved by usual `String.legth()` method.

There is already some pull request 
[https://github.com/apache/commons-lang/pull/332] but I decided to create the 
ticket because we also need to support masking for @ToString annotation and I 
would like to propose [PR with my own implementation of mask() 
function|https://github.com/apache/commons-lang/pull/335].


 


> StringUtils: Add method for masking strings
> ---
>
> Key: LANG-1400
> URL: https://issues.apache.org/jira/browse/LANG-1400
> Project: Commons Lang
>  Issue Type: 

[jira] [Commented] (LANG-1400) StringUtils: Add method for masking strings

2018-06-23 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/LANG-1400?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16521300#comment-16521300
 ] 

ASF GitHub Bot commented on LANG-1400:
--

Github user stokito commented on the issue:

https://github.com/apache/commons-lang/pull/332
  
Hi @greenman18523 thank for your contribution - from my experience this is 
"must have" functionality for Commons Lang library because in almost all bit 
projects what I saw was their home grown masking function.

But what I would like to propose is to simplify the api and use one 
function `mask()` instead of two `maskStart()` and `maskEnd()`. This will 
simplify code of the functions especially both of them are quite similar. I 
created a pull request with my alternative implementation #335
Also I created a JIRA ticket 
https://issues.apache.org/jira/browse/LANG-1400 

Please give your feedback,
Thank you



> StringUtils: Add method for masking strings
> ---
>
> Key: LANG-1400
> URL: https://issues.apache.org/jira/browse/LANG-1400
> Project: Commons Lang
>  Issue Type: Improvement
>  Components: lang.*
>Reporter: Sergey Ponomarev
>Priority: Minor
>
> It would be great to have a function which allows to mask the original str by 
> replacing it's
> characters with the specified character-mask. For eaxmple to mask credit card:
> {code}
> mask("3566002020360505", 4, 4, '*') = "35660505"
> {code}
> Thus the number was hidden by asterisks while first and last four digits are 
> unmasked and seen.
> Common use case is to hide sensitive information from logs, by using it in 
> toString() of classes or in inputs to log calls.
> I think this is "must have" functionality for Commons Lang library because 
> from my experience in almost all bit projects what I saw was their home grown 
> masking function.
> I think this is very important also because this is required for masking 
> credit card numbers by PCI compliance. Also new GDPR rules requires to hide 
> personal info as much as possible so masking of First and Last names now is 
> required by laws.
> To make the world safer place the utility for masking should be already 
> existing in platform to avoid situations when developer think  "this is 
> better to mask but I don't have enough time now so let's do this latter" 
> until leak happens.
> IMHO this should be implemented even in `String` class itself.
> From my experience what I saw was usually few masking usages and styles:
> 1. masking of passwords and names: only first and last symbols are shown, 
> mask char is `*`
> 2. masking of credit cards: only first and last 4 or 6 symbols are shown, 
> mask char is `*`.
> 3. credit card number shortest masking of last symbols i.e. 
> `mask("4242424242424242") == " *4242"` but it's not so often used, I just 
> wanted to mention.
> 4. not masking but showing a length just so see that value was passed. This 
> can be easily achieved by usual `String.legth()` method.
> There is already some pull request 
> [https://github.com/apache/commons-lang/pull/332] but I decided to create the 
> ticket because we also need to support masking for @ToString annotation and I 
> would like to propose [PR with my own implementation of mask() 
> function|https://github.com/apache/commons-lang/pull/335].
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] commons-lang issue #332: Add methods allowing masking of Strings

2018-06-23 Thread stokito
Github user stokito commented on the issue:

https://github.com/apache/commons-lang/pull/332
  
Hi @greenman18523 thank for your contribution - from my experience this is 
"must have" functionality for Commons Lang library because in almost all bit 
projects what I saw was their home grown masking function.

But what I would like to propose is to simplify the api and use one 
function `mask()` instead of two `maskStart()` and `maskEnd()`. This will 
simplify code of the functions especially both of them are quite similar. I 
created a pull request with my alternative implementation #335
Also I created a JIRA ticket 
https://issues.apache.org/jira/browse/LANG-1400 

Please give your feedback,
Thank you



---


[jira] [Updated] (LANG-1400) StringUtils: Add method for masking strings

2018-06-23 Thread Sergey Ponomarev (JIRA)


 [ 
https://issues.apache.org/jira/browse/LANG-1400?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Sergey Ponomarev updated LANG-1400:
---
Description: 
It would be great to have a function which allows to mask the original str by 
replacing it's
characters with the specified character-mask. For eaxmple to mask credit card:
{code}
mask("3566002020360505", 4, 4, '*') = "35660505"
{code}
Thus the number was hidden by asterisks while first and last four digits are 
unmasked and seen.

Common use case is to hide sensitive information from logs, by using it in 
toString() of classes or in inputs to log calls.

I think this is "must have" functionality for Commons Lang library because from 
my experience in almost all bit projects what I saw was their home grown 
masking function.
I think this is very important also because this is required for masking credit 
card numbers by PCI compliance. Also new GDPR rules requires to hide personal 
info as much as possible so masking of First and Last names now is required by 
laws.
To make the world safer place the utility for masking should be already 
existing in platform to avoid situations when developer think  "this is better 
to mask but I don't have enough time now so let's do this latter" until leak 
happens.
IMHO this should be implemented even in `String` class itself.

>From my experience what I saw was usually few masking usages and styles:
1. masking of passwords and names: only first and last symbols are shown, mask 
char is `*`
2. masking of credit cards: only first and last 4 or 6 symbols are shown, mask 
char is `*`.
3. credit card number shortest masking of last symbols i.e. 
`mask("4242424242424242") == " *4242"` but it's not so often used, I just 
wanted to mention.
4. not masking but showing a length just so see that value was passed. This can 
be easily achieved by usual `String.legth()` method.

There is already some pull request 
[https://github.com/apache/commons-lang/pull/332] but I decided to create the 
ticket because we also need to support masking for @ToString annotation and I 
would like to propose [PR with my own implementation of mask() 
function|https://github.com/apache/commons-lang/pull/335].


 

  was:
It would be great to have a function which allows to mask the original str by 
replacing it's
characters with the specified character-mask. For eaxmple to mask credit card:
{code}
mask("3566002020360505", 4, 4, '*') = "35660505"
{code}
Thus the number was hidden by asterisks while first and last four digits are 
unmasked and seen.

Common use case is to hide sensitive information from logs, by using it in 
toString() of classes or in inputs to log calls.

I think this is "must have" functionality for Commons Lang library because from 
my experience in almost all bit projects what I saw was their home grown 
masking function.
I think this is very important also because this is required for masking credit 
card numbers by PCI compliance. Also new GDPR rules requires to hide personal 
info as much as possible so masking of First and Last names now is required by 
laws.
To make the world safer place the utility for masking should be already 
existing in platform to avoid situations when developer think  "this is better 
to mask but I don't have enough time now so let's do this latter" until leak 
happens.
IMHO this should be implemented even in `String` class itself.

>From my experience what I saw was usually few masking usages and styles:
1. masking of passwords and names: only first and last symbols are shown, mask 
char is `*`
2. masking of credit cards: only first and last 4 or 6 symbols are shown, mask 
char is `*`.
3. credit card number shortest masking of last symbols i.e. 
`mask("4242424242424242") == " *4242"` but it's not so often used, I just 
wanted to mention.
4. not masking but showing a length just so see that value was passed. This can 
be easily achieved by usual `String.legth()` method.

There is already some pull request 
[https://github.com/apache/commons-lang/pull/332] but I decided to create the 
ticket because we also need to support masking for @ToString annotation and I 
would like to propose my own implementation of mask() function.


 


> StringUtils: Add method for masking strings
> ---
>
> Key: LANG-1400
> URL: https://issues.apache.org/jira/browse/LANG-1400
> Project: Commons Lang
>  Issue Type: Improvement
>  Components: lang.*
>Reporter: Sergey Ponomarev
>Priority: Minor
>
> It would be great to have a function which allows to mask the original str by 
> replacing it's
> characters with the specified character-mask. For eaxmple to mask credit card:
> {code}
> mask("3566002020360505", 4, 4, '*') = "35660505"
> {code}
> Thus the number was hidden by asterisks while first and last four digits are 
> 

[jira] [Commented] (LANG-1400) StringUtils: Add method for masking strings

2018-06-23 Thread ASF GitHub Bot (JIRA)


[ 
https://issues.apache.org/jira/browse/LANG-1400?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16521296#comment-16521296
 ] 

ASF GitHub Bot commented on LANG-1400:
--

GitHub user stokito opened a pull request:

https://github.com/apache/commons-lang/pull/335

LANG-1400: Add StringUtils.mask() function

My alternative implementation for #332
See https://issues.apache.org/jira/browse/LANG-1400 for details

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/stokito/commons-lang LANG-1400

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/commons-lang/pull/335.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #335


commit 001b769fab526f2034c155dab422dfe10bf48cff
Author: Sergey Ponomarev 
Date:   2018-06-23T22:47:33Z

LANG-1400: Add StringUtils.mask() function




> StringUtils: Add method for masking strings
> ---
>
> Key: LANG-1400
> URL: https://issues.apache.org/jira/browse/LANG-1400
> Project: Commons Lang
>  Issue Type: Improvement
>  Components: lang.*
>Reporter: Sergey Ponomarev
>Priority: Minor
>
> It would be great to have a function which allows to mask the original str by 
> replacing it's
> characters with the specified character-mask. For eaxmple to mask credit card:
> {code}
> mask("3566002020360505", 4, 4, '*') = "35660505"
> {code}
> Thus the number was hidden by asterisks while first and last four digits are 
> unmasked and seen.
> Common use case is to hide sensitive information from logs, by using it in 
> toString() of classes or in inputs to log calls.
> I think this is "must have" functionality for Commons Lang library because 
> from my experience in almost all bit projects what I saw was their home grown 
> masking function.
> I think this is very important also because this is required for masking 
> credit card numbers by PCI compliance. Also new GDPR rules requires to hide 
> personal info as much as possible so masking of First and Last names now is 
> required by laws.
> To make the world safer place the utility for masking should be already 
> existing in platform to avoid situations when developer think  "this is 
> better to mask but I don't have enough time now so let's do this latter" 
> until leak happens.
> IMHO this should be implemented even in `String` class itself.
> From my experience what I saw was usually few masking usages and styles:
> 1. masking of passwords and names: only first and last symbols are shown, 
> mask char is `*`
> 2. masking of credit cards: only first and last 4 or 6 symbols are shown, 
> mask char is `*`.
> 3. credit card number shortest masking of last symbols i.e. 
> `mask("4242424242424242") == " *4242"` but it's not so often used, I just 
> wanted to mention.
> 4. not masking but showing a length just so see that value was passed. This 
> can be easily achieved by usual `String.legth()` method.
> There is already some pull request 
> [https://github.com/apache/commons-lang/pull/332] but I decided to create the 
> ticket because we also need to support masking for @ToString annotation and I 
> would like to propose my own implementation of mask() function.
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] commons-lang pull request #335: LANG-1400: Add StringUtils.mask() function

2018-06-23 Thread stokito
GitHub user stokito opened a pull request:

https://github.com/apache/commons-lang/pull/335

LANG-1400: Add StringUtils.mask() function

My alternative implementation for #332
See https://issues.apache.org/jira/browse/LANG-1400 for details

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/stokito/commons-lang LANG-1400

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/commons-lang/pull/335.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #335


commit 001b769fab526f2034c155dab422dfe10bf48cff
Author: Sergey Ponomarev 
Date:   2018-06-23T22:47:33Z

LANG-1400: Add StringUtils.mask() function




---


[jira] [Created] (LANG-1400) StringUtils: Add method for masking strings

2018-06-23 Thread Sergey Ponomarev (JIRA)
Sergey Ponomarev created LANG-1400:
--

 Summary: StringUtils: Add method for masking strings
 Key: LANG-1400
 URL: https://issues.apache.org/jira/browse/LANG-1400
 Project: Commons Lang
  Issue Type: Improvement
  Components: lang.*
Reporter: Sergey Ponomarev


It would be great to have a function which allows to mask the original str by 
replacing it's
characters with the specified character-mask. For eaxmple to mask credit card:
{code}
mask("3566002020360505", 4, 4, '*') = "35660505"
{code}
Thus the number was hidden by asterisks while first and last four digits are 
unmasked and seen.

Common use case is to hide sensitive information from logs, by using it in 
toString() of classes or in inputs to log calls.

I think this is "must have" functionality for Commons Lang library because from 
my experience in almost all bit projects what I saw was their home grown 
masking function.
I think this is very important also because this is required for masking credit 
card numbers by PCI compliance. Also new GDPR rules requires to hide personal 
info as much as possible so masking of First and Last names now is required by 
laws.
To make the world safer place the utility for masking should be already 
existing in platform to avoid situations when developer think  "this is better 
to mask but I don't have enough time now so let's do this latter" until leak 
happens.
IMHO this should be implemented even in `String` class itself.

>From my experience what I saw was usually few masking usages and styles:
1. masking of passwords and names: only first and last symbols are shown, mask 
char is `*`
2. masking of credit cards: only first and last 4 or 6 symbols are shown, mask 
char is `*`.
3. credit card number shortest masking of last symbols i.e. 
`mask("4242424242424242") == " *4242"` but it's not so often used, I just 
wanted to mention.
4. not masking but showing a length just so see that value was passed. This can 
be easily achieved by usual `String.legth()` method.

There is already some pull request 
[https://github.com/apache/commons-lang/pull/332] but I decided to create the 
ticket because we also need to support masking for @ToString annotation and I 
would like to propose my own implementation of mask() function.


 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[GitHub] commons-lang issue #318: clean code

2018-06-23 Thread stokito
Github user stokito commented on the issue:

https://github.com/apache/commons-lang/pull/318
  
Changes are ok


---


[GitHub] commons-lang issue #278: Lang-1345 Enhance non-empty strings

2018-06-23 Thread stokito
Github user stokito commented on the issue:

https://github.com/apache/commons-lang/pull/278
  
For me is not clear importance of the new methods. I can't remember 
situations when I needed for something like this. And even if I need something 
like this I'll write the function myself and I'll not even think about to 
search something in Commons Lang library. Time spent to looking for the method 
will be ten time more then write myself.
Maybe commons lang itself uses the function internally then it can make 
sense.

It would be great if you can give us an examples of usage and how they are 
important.


---


[GitHub] commons-lang issue #334: Code refactoring and cleaning

2018-06-23 Thread stokito
Github user stokito commented on the issue:

https://github.com/apache/commons-lang/pull/334
  
I checked and everything is ok in terms of backward compatibility. But the 
commit 7f8571a506c7081bae0cf27bd93295e0344160bf "flips the order of conditional 
expressions and 'if' statements whose conditions were negated." may be not so 
good in terms of code readability.
First of all, there is always some natural flow when we expect that 
something is more likely happens.
Let's take a look on the method `addProcessor()`:
```java
private static void addProcessor(final String key, final Processor 
processor) {
if (!ARCH_TO_PROCESSOR.containsKey(key)) {
ARCH_TO_PROCESSOR.put(key, processor);
} else {
final String msg = "Key " + key + " already exists in processor 
map";
throw new IllegalStateException(msg);
}
}
```
Here developer sees a "happy path" first and only then he or she sees 
exception situation handling.
This is more natural to understand. As I remember this style was mentioned 
in Complete Code book.

Also this style has slightly better performance because in most situations 
processor will go forward to prefetched instructions or even it can be already 
executed by [branch prediction](https://en.wikipedia.org/wiki/Branch_predictor).
Meanwhile removing of negation usually doesn't have any performance impact 
because processors have already negated instructions.

I thinks it's ok to merge this commit but I just wan't you to know that 
sometimes it is better to make unnecessary negation.


---