Skip to content

Commit

Permalink
create local copies of realThing to avoid TOCTOU race condition
Browse files Browse the repository at this point in the history
  • Loading branch information
bodewig committed May 22, 2021
1 parent e6ab7d0 commit 96fa99e
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 23 deletions.
3 changes: 3 additions & 0 deletions WHATSNEW
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ Changes from Ant 1.10.10 TO Ant 1.10.11
Fixed bugs:
-----------

* a race condition could lead to NullPointerExceptions when running
tasks in parallel.
Bugzilla Report 65316

Other changes:
--------------
Expand Down
55 changes: 32 additions & 23 deletions src/main/org/apache/tools/ant/UnknownElement.java
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ public RuntimeConfigurable getWrapper() {
* @exception BuildException if the configuration fails
*/
public void maybeConfigure() throws BuildException {
if (realThing != null) {
final Object copy = realThing;
if (copy != null) {
return;
}
configure(makeObject(this, getWrapper()));
Expand All @@ -177,18 +178,18 @@ public void configure(Object realObject) {
}
realThing = realObject;

getWrapper().setProxy(realThing);
getWrapper().setProxy(realObject);
Task task = null;
if (realThing instanceof Task) {
task = (Task) realThing;
if (realObject instanceof Task) {
task = (Task) realObject;

task.setRuntimeConfigurableWrapper(getWrapper());

// For Script example that modifies id'ed tasks in other
// targets to work. *very* Ugly
// The reference is replaced by RuntimeConfigurable
if (getWrapper().getId() != null) {
this.getOwningTarget().replaceChild(this, (Task) realThing);
this.getOwningTarget().replaceChild(this, (Task) realObject);
}
}

Expand All @@ -203,7 +204,7 @@ public void configure(Object realObject) {
getWrapper().maybeConfigure(getProject());
}

handleChildren(realThing, getWrapper());
handleChildren(realObject, getWrapper());
}

/**
Expand All @@ -212,8 +213,9 @@ public void configure(Object realObject) {
* @param output The output to log. Should not be <code>null</code>.
*/
protected void handleOutput(String output) {
if (realThing instanceof Task) {
((Task) realThing).handleOutput(output);
final Object copy = realThing;
if (copy instanceof Task) {
((Task) copy).handleOutput(output);
} else {
super.handleOutput(output);
}
Expand All @@ -233,8 +235,9 @@ protected void handleOutput(String output) {
*/
protected int handleInput(byte[] buffer, int offset, int length)
throws IOException {
if (realThing instanceof Task) {
return ((Task) realThing).handleInput(buffer, offset, length);
final Object copy = realThing;
if (copy instanceof Task) {
return ((Task) copy).handleInput(buffer, offset, length);
}
return super.handleInput(buffer, offset, length);
}
Expand All @@ -245,8 +248,9 @@ protected int handleInput(byte[] buffer, int offset, int length)
* @param output The output to log. Should not be <code>null</code>.
*/
protected void handleFlush(String output) {
if (realThing instanceof Task) {
((Task) realThing).handleFlush(output);
final Object copy = realThing;
if (copy instanceof Task) {
((Task) copy).handleFlush(output);
} else {
super.handleFlush(output);
}
Expand All @@ -258,8 +262,9 @@ protected void handleFlush(String output) {
* @param output The error output to log. Should not be <code>null</code>.
*/
protected void handleErrorOutput(String output) {
if (realThing instanceof Task) {
((Task) realThing).handleErrorOutput(output);
final Object copy = realThing;
if (copy instanceof Task) {
((Task) copy).handleErrorOutput(output);
} else {
super.handleErrorOutput(output);
}
Expand All @@ -271,8 +276,9 @@ protected void handleErrorOutput(String output) {
* @param output The error output to log. Should not be <code>null</code>.
*/
protected void handleErrorFlush(String output) {
if (realThing instanceof Task) {
((Task) realThing).handleErrorFlush(output);
final Object copy = realThing;
if (copy instanceof Task) {
((Task) copy).handleErrorFlush(output);
} else {
super.handleErrorFlush(output);
}
Expand All @@ -283,13 +289,14 @@ protected void handleErrorFlush(String output) {
* (e.g. a data type) then this method does nothing.
*/
public void execute() {
if (realThing == null) {
final Object copy = realThing;
if (copy == null) {
// Got here if the runtimeconfigurable is not enabled.
return;
}
try {
if (realThing instanceof Task) {
((Task) realThing).execute();
if (copy instanceof Task) {
((Task) copy).execute();
}
} finally {
// Finished executing the task
Expand Down Expand Up @@ -503,8 +510,9 @@ protected BuildException getNotFoundException(String what,
* @return the name to use in logging messages.
*/
public String getTaskName() {
return !(realThing instanceof Task) ? super.getTaskName()
: ((Task) realThing).getTaskName();
final Object copy = realThing;
return !(copy instanceof Task) ? super.getTaskName()
: ((Task) copy).getTaskName();
}

/**
Expand All @@ -514,8 +522,9 @@ public String getTaskName() {
* a task.
*/
public Task getTask() {
if (realThing instanceof Task) {
return (Task) realThing;
final Object copy = realThing;
if (copy instanceof Task) {
return (Task) copy;
}
return null;
}
Expand Down

0 comments on commit 96fa99e

Please sign in to comment.