Skip to main content
added 4 characters in body
Source Link
mhmxs
  • 344
  • 1
  • 5

First of all, this code is not thread safe. If different threads find the privateKey as null, each of them will generate a different value into it. You should use "static constructor" or have to use Double-checked locking.

From security reason I didn't see any problem, but the code itself is not following object oriented paradigm. You should avoid static fields and methods, except in utility classes (there are many developer, who suggestssuggest to avoid "static only classes" totally).

I guess setPrivateKey(String privateKey) should be private.

I didn' find the code where you convert + back.

I'm not sure, so a question: BasicTextEncryptor could decrypt the hash if the privateKey is different than the encrypt one?

Instead of:

HashMap<String, String> out = new HashMap<String, String>();

    //cookies in the map?
    if (cookies == null || cookies.isEmpty()) {
        return out;
    }

you should use:

    if (cookies == null || cookies.isEmpty()) {
        return Collections.emptyMap();
    }
    HashMap<String, String> out = new HashMap<String, String>();

First of all, this code is not thread safe. If different threads find the privateKey as null, each of them will generate a different value into it. You should use "static constructor" or have to use Double-checked locking.

From security reason I didn't see any problem, but the code itself is not following object oriented paradigm. You should avoid static fields and methods, except in utility classes (there are many developer, who suggests to avoid "static classes" totally).

I guess setPrivateKey(String privateKey) should be private.

I didn' find the code where you convert + back.

I'm not sure, so a question: BasicTextEncryptor could decrypt the hash if the privateKey is different than the encrypt one?

Instead of:

HashMap<String, String> out = new HashMap<String, String>();

    //cookies in the map?
    if (cookies == null || cookies.isEmpty()) {
        return out;
    }

you should use:

    if (cookies == null || cookies.isEmpty()) {
        return Collections.emptyMap();
    }
    HashMap<String, String> out = new HashMap<String, String>();

First of all, this code is not thread safe. If different threads find the privateKey as null, each of them will generate a different value into it. You should use "static constructor" or have to use Double-checked locking.

From security reason I didn't see any problem, but the code itself is not following object oriented paradigm. You should avoid static fields and methods, except in utility classes (there are many developer, who suggest to avoid "static only classes" totally).

I guess setPrivateKey(String privateKey) should be private.

I didn' find the code where you convert + back.

I'm not sure, so a question: BasicTextEncryptor could decrypt the hash if the privateKey is different than the encrypt one?

Instead of:

HashMap<String, String> out = new HashMap<String, String>();

    //cookies in the map?
    if (cookies == null || cookies.isEmpty()) {
        return out;
    }

you should use:

    if (cookies == null || cookies.isEmpty()) {
        return Collections.emptyMap();
    }
    HashMap<String, String> out = new HashMap<String, String>();
Source Link
mhmxs
  • 344
  • 1
  • 5

First of all, this code is not thread safe. If different threads find the privateKey as null, each of them will generate a different value into it. You should use "static constructor" or have to use Double-checked locking.

From security reason I didn't see any problem, but the code itself is not following object oriented paradigm. You should avoid static fields and methods, except in utility classes (there are many developer, who suggests to avoid "static classes" totally).

I guess setPrivateKey(String privateKey) should be private.

I didn' find the code where you convert + back.

I'm not sure, so a question: BasicTextEncryptor could decrypt the hash if the privateKey is different than the encrypt one?

Instead of:

HashMap<String, String> out = new HashMap<String, String>();

    //cookies in the map?
    if (cookies == null || cookies.isEmpty()) {
        return out;
    }

you should use:

    if (cookies == null || cookies.isEmpty()) {
        return Collections.emptyMap();
    }
    HashMap<String, String> out = new HashMap<String, String>();