[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareStor...

[rsafonseca]

Github user rsafonseca commented on the pull request:

https://github.com/apache/cloudstack/pull/460#issuecomment-112344047
  
@DaanHoogland done!
https://github.com/apache/cloudstack/pull/464
I should have checked travis for this one, still my bad :(


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareStor...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/460#issuecomment-112339045
  
please make it a seperate fix, not an update. I jumped the gun.

On Tue, Jun 16, 2015 at 10:36 AM, Rafael da Fonseca <
notificati...@github.com> wrote:

> my bad there.. will surround in try/catch block and update
>
> —
> Reply to this email directly or view it on GitHub
> .
>



-- 
Daan



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareStor...

[Daan Hoogland]

I think it's caught and also that the pull request builder doesn't
build -Dnoredist. I will check and fix if so

On Tue, Jun 16, 2015 at 10:36 AM, rsafonseca  wrote:
> Github user rsafonseca commented on the pull request:
>
> https://github.com/apache/cloudstack/pull/460#issuecomment-112338005
>
> my bad there.. will surround in try/catch block and update
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
> with INFRA.
> ---



-- 
Daan

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareStor...

[rsafonseca]

Github user rsafonseca commented on the pull request:

https://github.com/apache/cloudstack/pull/460#issuecomment-112338005
  
my bad there.. will surround in try/catch block and update


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareStor...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/460#issuecomment-112333921
  
I shouldn't have:( exception uncought. Really curious why the pull request 
builder accepted this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareStor...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/cloudstack/pull/460


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareStor...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/460#issuecomment-112313538
  
thanks, LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareReso...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/cloudstack/pull/462


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareReso...

[DaanHoogland]

Github user DaanHoogland commented on the pull request:

https://github.com/apache/cloudstack/pull/462#issuecomment-112313034
  
convinced, the router is a debian box (for now and will remain linux in the 
near future.

LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareStor...

[rsafonseca]

Github user rsafonseca commented on the pull request:

https://github.com/apache/cloudstack/pull/460#issuecomment-112232241
  
Here's an example of an instance where i also removed a 
Charset.defaultCharset() that was placed in the reading of a socket, where 
cloudstack controls both ends -> 
https://github.com/apache/cloudstack/pull/400/files
If ConsoleProxyCmdHandler.java would run on a different encoding, you might 
have issues, so i changes all classes that used this to use UTF-8 explicitly. 
Charset.defaultCharset() should still work on 
CitrixConsoleProxyLoadCommandWrapper.java (if this runs on the agent side, 
didn't check) since platform that Xenserver  runs on also uses UTF-8 as a 
default.. but in the off chance the sysadmin screwed with the java defaults or 
this isn't running on the agent side, it might then fail.. this ensures it 
would work properly no matter what, it's always safer to do this if you control 
both the reader and the writer.

And here's an example where i think it's most suited to use 
Charset.defaultCharset() , the output of a shell command:
https://github.com/apache/cloudstack/pull/396

It's also possible to screw with the output of this from the system's side 
(although very unlikely), but this is the best approximation of 100% fail-proof 
of getting proper encoding of the input data.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareReso...

[rsafonseca]

Github user rsafonseca commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/462#discussion_r32472663
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -705,7 +705,7 @@ public ExecutionResult createFileInVR(String routerIp, 
String filePath, String f
 VmwareManager mgr = 
getServiceContext().getStockObject(VmwareManager.CONTEXT_STOCK_NAME);
 File keyFile = mgr.getSystemVMKeyFile();
 try {
-SshHelper.scpTo(routerIp, 3922, "root", keyFile, null, 
filePath, content.getBytes(), fileName, null);
+SshHelper.scpTo(routerIp, 3922, "root", keyFile, null, 
filePath, content.getBytes("UTF-8"), fileName, null);
--- End diff --

VR runs on a UTF-8 platform.
If you're running this on windows and output a specific set of bytes based 
on windows' encoding, the result will be different when VR reads the file. 
UTF-8 should be used specifically in this case, since we're outputting a 
specific array of bytes to a platform with known encoding. 
Charset.defaultCharset() would only work well if the platorm it is running on 
matches that encoding which it actually is in most cases since most peeps run 
this on a modern linux distro ;)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareStor...

[rsafonseca]

Github user rsafonseca commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/460#discussion_r32469324
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
 ---
@@ -2265,7 +2265,7 @@ public Answer forgetObject(ForgetObjectCmd cmd) {
 }
 
 private static String deriveTemplateUuidOnHost(VmwareHypervisorHost 
hyperHost, String storeIdentifier, String templateName) {
-String templateUuid = UUID.nameUUIDFromBytes((templateName + "@" + 
storeIdentifier + "-" + hyperHost.getMor().getValue()).getBytes()).toString();
+String templateUuid = UUID.nameUUIDFromBytes((templateName + "@" + 
storeIdentifier + "-" + 
hyperHost.getMor().getValue()).getBytes("UTF-8")).toString();
--- End diff --

Unless you're getting platform specifc data, i think it's best to always 
stick to use the same charset, to avoid different values from being generated 
in different platforms whenever possible. Ideally, we should convert all data 
that comes into the application to a standard charset that supports 
internationalization such as UTF-8, and just use default platform charset where 
applicable, when consuming data from platform specific sources.
Using Charset.defaultCharset() will require an extra import and increase 
the overall size of the class, altough instantiating a String also has its own 
overhead, i believe it to be less so.. we can also remove string instantiation 
at runtime by defining a constant, which will also increase runtime efficiency.
Of course this is only my point of view, please comment if you feel 
otherwise :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareReso...

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/462#discussion_r32466095
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
 ---
@@ -705,7 +705,7 @@ public ExecutionResult createFileInVR(String routerIp, 
String filePath, String f
 VmwareManager mgr = 
getServiceContext().getStockObject(VmwareManager.CONTEXT_STOCK_NAME);
 File keyFile = mgr.getSystemVMKeyFile();
 try {
-SshHelper.scpTo(routerIp, 3922, "root", keyFile, null, 
filePath, content.getBytes(), fileName, null);
+SshHelper.scpTo(routerIp, 3922, "root", keyFile, null, 
filePath, content.getBytes("UTF-8"), fileName, null);
--- End diff --

please use Charset.defaultCharset() instead of "UTF8"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareStor...

[DaanHoogland]

Github user DaanHoogland commented on a diff in the pull request:

https://github.com/apache/cloudstack/pull/460#discussion_r32465788
  
--- Diff: 
plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
 ---
@@ -2265,7 +2265,7 @@ public Answer forgetObject(ForgetObjectCmd cmd) {
 }
 
 private static String deriveTemplateUuidOnHost(VmwareHypervisorHost 
hyperHost, String storeIdentifier, String templateName) {
-String templateUuid = UUID.nameUUIDFromBytes((templateName + "@" + 
storeIdentifier + "-" + hyperHost.getMor().getValue()).getBytes()).toString();
+String templateUuid = UUID.nameUUIDFromBytes((templateName + "@" + 
storeIdentifier + "-" + 
hyperHost.getMor().getValue()).getBytes("UTF-8")).toString();
--- End diff --

In other  cases I used Charset.defaultCharset(). Is there a reason to 
explicitly use "UTF8"?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareReso...

[rsafonseca]

GitHub user rsafonseca opened a pull request:

https://github.com/apache/cloudstack/pull/462

Fix findbugs encoding issue in VmwareResource.java

Key file should be UTF-8 encoded in VR

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

$ git pull https://github.com/rsafonseca/cloudstack findbugs77

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

https://github.com/apache/cloudstack/pull/462.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 #462


commit f02aaf83b05e836ea1f4739baa8e60af833f51b6
Author: Rafael da Fonseca 
Date:   2015-06-15T17:40:47Z

Fix findbugs encoding issue in VmwareResource.java
Key file should be UTF-8 encoded in VR




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in VmwareStor...

[rsafonseca]

GitHub user rsafonseca opened a pull request:

https://github.com/apache/cloudstack/pull/460

Fix findbugs encoding issue in VmwareStorageProcessor.java

Any encoding would do fine as it's just used to generate a UUID. Sticking 
with UTF-8 for consistency

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

$ git pull https://github.com/rsafonseca/cloudstack findbugs75

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

https://github.com/apache/cloudstack/pull/460.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 #460


commit 8cb5f19dcc1f6e0775ddfb4802f30e6463499958
Author: Rafael da Fonseca 
Date:   2015-06-15T17:15:55Z

Fix findbugs encoding issue in VmwareStorageProcessor.java
Any encoding would do fine as it's just used to generate a UUID. Sticking 
with UTF-8 for consistency




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in MemStat.ja...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/cloudstack/pull/402


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/cloudstack/pull/394


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue in MemStat.ja...

[rsafonseca]

GitHub user rsafonseca opened a pull request:

https://github.com/apache/cloudstack/pull/402

Fix findbugs encoding issue in MemStat.java

Was safe either way as this piece of code should only run in linux and only 
contains safe characters, this just gets rid of the findbugs warning

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

$ git pull https://github.com/rsafonseca/cloudstack findbugs19

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

https://github.com/apache/cloudstack/pull/402.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 #402


commit 75a6b90705da7752060d1611fa6eee12f44ee63d
Author: Rafael da Fonseca 
Date:   2015-06-13T22:04:44Z

Was safe either way as this piece of code should only run in linux, this 
just gets rid of the findbugs warning




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request: Fix findbugs encoding issue

[rsafonseca]

GitHub user rsafonseca opened a pull request:

https://github.com/apache/cloudstack/pull/394

Fix findbugs encoding issue

This is done by calling HttpMethodBase's getResponseBodyAsString() which 
properly looks up the specified encoding in the request's Content-Type header
This also avoids instantiation of two extra strings on the println() and 
return statements

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

$ git pull https://github.com/rsafonseca/cloudstack findbugs10

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

https://github.com/apache/cloudstack/pull/394.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 #394


commit 315b75e4ff9d324cde8c4156796ef9e382f60369
Author: Rafael da Fonseca 
Date:   2015-06-12T20:07:20Z

Fix findbugs encoding issue
This is done by calling HttpMethodBase's getResponseBodyAsString() which 
properly lookup the specified encoding in the request's Content-Type header
This also avoids instantiation of two extra strings on the println() and 
return statements




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---