You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by quaff <gi...@git.apache.org> on 2015/02/15 06:36:48 UTC
[GitHub] struts pull request: Improve LocalizedTextUtil depend NPE
GitHub user quaff opened a pull request:
https://github.com/apache/struts/pull/35
Improve LocalizedTextUtil depend NPE
depend NPE if not ValueStack presented, for example non-struts request or backend thread.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/quaff/struts develop
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/struts/pull/35.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 #35
----
commit 86195acc82b9652c09c6390bc85ea2a82d04acf8
Author: zhouyanming <zh...@gmail.com>
Date: 2015-02-15T05:35:00Z
Improve LocalizedTextUtil depend NPE
depend NPE if not ValueStack presented, for example non-struts request or backend thread.
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by quaff <gi...@git.apache.org>.
Github user quaff commented on the pull request:
https://github.com/apache/struts/pull/35#issuecomment-77827292
@lukaszlenart , It doesn't add new thing just null-check added, It's safe if existing unit tests passed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by quaff <gi...@git.apache.org>.
Github user quaff commented on the pull request:
https://github.com/apache/struts/pull/35#issuecomment-77971388
ValueStack will be always not null with struts2 request, so It wouldn't change the behavior of struts2, this pull request is intend to use in non-struts2 context.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by lukaszlenart <gi...@git.apache.org>.
Github user lukaszlenart commented on the pull request:
https://github.com/apache/struts/pull/35#issuecomment-78003822
That's the case, can you explain what you mean by non-struts2 context?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by lukaszlenart <gi...@git.apache.org>.
Github user lukaszlenart commented on the pull request:
https://github.com/apache/struts/pull/35#issuecomment-78242017
So you can use a `StubValueContext` like this one
```java
public class StubValueStack implements ValueStack {
Map<String, Object> ctx = new HashMap<String, Object>();
CompoundRoot root = new CompoundRoot();
public Map<String, Object> getContext() {
return ctx;
}
public void setDefaultType(Class defaultType) {
}
public void setExprOverrides(Map<Object, Object> overrides) {
}
public Map<Object, Object> getExprOverrides() {
return null;
}
public CompoundRoot getRoot() {
return root;
}
public void setValue(String expr, Object value) {
ctx.put(expr, value);
}
public void setParameter(String expr, Object value) {
throw new UnsupportedOperationException("not implemented");
}
public void setValue(String expr, Object value, boolean throwExceptionOnFailure) {
ctx.put(expr, value);
}
public String findString(String expr) {
return (String) ctx.get(expr);
}
public String findString(String expr, boolean throwExceptionOnFailure) {
return findString(expr, false);
}
public Object findValue(String expr) {
return findValue(expr, false);
}
public Object findValue(String expr, boolean throwExceptionOnFailure) {
return ctx.get(expr);
}
public Object findValue(String expr, Class asType) {
return findValue(expr, asType, false);
}
public Object findValue(String expr, Class asType, boolean throwExceptionOnFailure) {
return ctx.get(expr);
}
public Object peek() {
return root.peek();
}
public Object pop() {
return root.pop();
}
public void push(Object o) {
root.push(o);
}
public void set(String key, Object o) {
ctx.put(key, o);
}
public int size() {
return root.size();
}
}
```
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by lukaszlenart <gi...@git.apache.org>.
Github user lukaszlenart commented on the pull request:
https://github.com/apache/struts/pull/35#issuecomment-74625231
I'm not sure if this is a good solution, can you provide more details?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/struts/pull/35
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by quaff <gi...@git.apache.org>.
Github user quaff commented on the pull request:
https://github.com/apache/struts/pull/35#issuecomment-78400068
It's just a static util class, It should be light and simple, your suggestion is opposite.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by lukaszlenart <gi...@git.apache.org>.
Github user lukaszlenart commented on the pull request:
https://github.com/apache/struts/pull/35#issuecomment-77833815
What I meant is that if `ValueStack` is missing in context something wrong happened, so solution to null-check is error-prone :)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by quaff <gi...@git.apache.org>.
Github user quaff commented on the pull request:
https://github.com/apache/struts/pull/35#issuecomment-78184739
simplest case
```java
import java.util.Locale;
public class Main {
public static void main(String[] args) {
System.out.println(LocalizedTextUtil.findText(Main.class, "key",Locale.getDefault(), "defaultMessage", null, null));
}
}
```
Main.properties
```
key=this is a key
```
it will print "this is a key"
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by wolpi <gi...@git.apache.org>.
Github user wolpi commented on the pull request:
https://github.com/apache/struts/pull/35#issuecomment-74502592
could you add a unit test, please?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by quaff <gi...@git.apache.org>.
Github user quaff commented on the pull request:
https://github.com/apache/struts/pull/35#issuecomment-78441177
I hope so, please add a method drop last two arguments, thanks.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by lukaszlenart <gi...@git.apache.org>.
Github user lukaszlenart commented on the pull request:
https://github.com/apache/struts/pull/35#issuecomment-104374138
Please re-open this PR against `master` branch
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org
[GitHub] struts pull request: Improve LocalizedTextUtil defend NPE
Posted by lukaszlenart <gi...@git.apache.org>.
Github user lukaszlenart commented on the pull request:
https://github.com/apache/struts/pull/35#issuecomment-78426250
But you cannot pass `null` and expect that it will work properly, if you want such behaviour add method with signature that doesn't expect `ValueStack`
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org