Java and substring error

Question

I am implementing a method to encrypt with a key and i made a call like this:

Crypto c = new Crypto("mysecretkey");
String enc = c.encrypt("mytext");

But i am getting an exception

"crypto encrypt error: String index out of range: -1"

at this part:

String sKeyChar = getKey().substring((i % getKey().length()) - 1, 1);

And I don't know what I am doing wrong because I made the same thing in PHP and works good. Maybe this is simple but I am stuck, this is my method:

public String encrypt(String sData) {
        String sEncrypted = null;
        try {
            String sResult = null;
            for (int i = 0; i < sData.length(); i++) {
                String sChar = sData.substring(i, 1);
                String sKeyChar = getKey().substring((i % getKey().length()) - 1, 1);
                char c = (char) (ord(sChar) - ord(sKeyChar));
                String sPart = (new StringBuffer().append(c)).toString();
                sResult += sPart;
            }
            byte[] sResultBuff = sResult.getBytes("UTF-8");
            sEncrypted = Base64.encode(sResultBuff);
        } catch (Exception e) {
            System.out.println("crypto encrypt error: " + e.getMessage());
            sEncrypted = null;
        }
        return sEncrypted;
    }

Other method needed:

public int ord(String sChar) {
    int ascii_code = 0;
    try {
        ascii_code = String.valueOf(sChar.charAt(0)).codePointAt(0);
    } catch (Exception e) {
        System.out.println("crypto ord error: " + e.getMessage());
        ascii_code = 0;
    }
    return ascii_code;
}

PHP equivalent method:

function encrypt($sData, $sKey='mysecretkey'){ 
    $sResult = ''; 
    for($i=0;$i<strlen($sData);$i++){ 
        $sChar    = substr($sData, $i, 1); 
        $sKeyChar = substr($sKey, ($i % strlen($sKey)) - 1, 1); 
        $sChar    = chr(ord($sChar) + ord($sKeyChar)); 
        $sResult .= $sChar; 
    } 
    return encode_base64($sResult); 
}

Thanks!

You should post getKey() method as well, as it is used in line, which fails. — Mateusz Chromiński
– Mateusz Chromiński, Commented Jan 30, 2012 at 22:49
Am I wrong in thinking this isn't cryptographically secure? I feel like I should be able to divine the key and the data from this, knowing the method (supposing short keys). And that's assuming that the result of the subtraction operation is a valid char, anyways; that ord() method doesn't always return valid data, and isn't international safe (combined characters). Also, you're swallowing Exceptions, which is bad, and simply printing to System.out (and not even .err!), instead of properly logging. And look up StringBuilder - your implementation is not optimal. — Clockwork-Muse
– Clockwork-Muse, Commented Jan 30, 2012 at 23:22
Thanks for recomendations but this is just a test method and getKey() works fine — Oscar Jara
– Oscar Jara, Commented Jan 31, 2012 at 1:06

Thomas · Accepted Answer · 2012-01-30 22:55:54Z

2

Your calculation is wrong: (i % getKey().length()) - 1 will result in -1 for i = 0, i.e. right in the first iteration. Thus you try to pass -1 to the substring(...) method, which is not allowed.

Also note that if the data is longer than the key, i % getKey().length() will result in 0 for every multiple of the key length.

Additionally, the parameters to substring(...) are not index and length but startIndex (inclusive) and endIndex (exclusive). Thus String sChar = sData.substring(i, 1); will throw an exception once i reaches 2 (and above) and won't return anything for i = 1.

You might want to use charAt(i) instead (and getKey().charAt(i % getKey().length()) in the following line). Note that this returns a single character, which would make the ord(...) method obsolete.

As a side note: String.valueOf(sChar.charAt(0)).codePointAt(0) is equivalent to sChar.codePointAt(0).

Another side note:

char c = (char) (ord(sChar) - ord(sKeyChar));
String sPart = (new StringBuffer().append(c)).toString();
sResult += sPart;

can be simplified to

char c = (char) (ord(sChar) - ord(sKeyChar));
sResult += c; //you could also merge those two lines

edited Jan 30, 2012 at 22:55

answered Jan 30, 2012 at 22:42

Thomas

88.9k13 gold badges126 silver badges162 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Thomas Over a year ago

If someone's going to rate me down, it would be nice to explain why.

Sergey Kalinichenko · Accepted Answer · 2012-01-30 23:12:20Z

1

You see the difference between PHP and Java because PHP's substr understands negative numbers, but Java's substring does not: it throws an exception.

In PHP, passing negative 1 to substr means "get me the last character", but in Java you need to pass the index of the last character (i.e. str.length()-1) to achieve the same effect.

If this is not a mistake, and this is precisely the effect that you wanted to achieve, you can address this issue with an if condition:

int pos = (i % getKey().length()) - 1;
if (pos == -1) {
    pos = getKey().length() - 1;
}
// EDIT: Second argument needs to be pos+1, not 1. Thomas pointed out this error
String sKeyChar = getKey().substring(pos, pos+1);

EDIT As Thomas correctly pointed out, the other difference between PHP version of substr and Java's substring is in their treatment of the second argument: PHP thinks it's length; Java thinks it's the index of the last character plus one.

edited Jan 30, 2012 at 23:12

answered Jan 30, 2012 at 22:47

Sergey Kalinichenko

729k85 gold badges1.2k silver badges1.6k bronze badges

3 Comments

Thomas Over a year ago

That would produce an exception if pos was not 0 or 1.

Sergey Kalinichenko Over a year ago

@Thomas You are absolutely right, that's the other difference between PHP and Java. At this point I think your solution (going with individual characters rather than one-character strings) is a lot cleaner, so I'm upvoting it. Thanks for finding this bug!

Oscar Jara Over a year ago

Thanks for the solution :) I still getting an error at this line "char c = (char) (ord(sChar) - ord(sKeyChar));" in the ord() method but I will continue looking for what is happening. But everything clear, thanks again!

wmz · Accepted Answer · 2012-01-30 23:21:11Z

0

As already noted, substr in PHP has different semantics that in Java. In PHP its

string substr ( string $string , int $start [, int $length ] )
and it also accepts negative numbers (which alter it's behaviour),

while in java it's defined as:

 String     substring(int beginIndex, int endIndex)

and throws exception on negative numbers or if beginIndex is greater than endIndex. To summarize, they are different and you need to compensate for that.

answered Jan 30, 2012 at 23:21

wmz

3,6851 gold badge16 silver badges23 bronze badges

Collectives™ on Stack Overflow

Java and substring error

3 Answers 3

1 Comment

3 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related