Skip to content

Commit

Permalink
httpd: fix three small null-related bugs in accessing active transfer…
Browse files Browse the repository at this point in the history
… fields

Motivation:

Patch 9258 introduced a UserInfo object and also somewhat restructured TransferObserverV1.
In so doing it introduced two minor bugs.  The first was an erroneous suppression
of a condition preventing the addition of nulls to an argument list in the creation
of the ascii context info.  The latter has to do with the handling of exceptions thrown by
the Subjects static methods (this was observed to occur using anonymous dccp).

Also, a method called by the webadmin version of this service also neglected to
check for a null value.

Modification:

1.  Restore the condition check (mover == null) for the ascii context info.
2.  Provide for the handling of NoSuchElement and IllegalArgument exceptions
3.  Handle the null value for bytesTransferred.

Result:

Errors are now handled gracefully rather than causing Wicket to crash or a log
full of stack traces.

Target: master
Require-notes: no
Require-book: no
Acked-by: Gerd
  • Loading branch information
alrossi committed May 24, 2016
1 parent 4236e4a commit c39133d
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 15 deletions.
2 changes: 1 addition & 1 deletion modules/common/src/main/java/org/dcache/auth/Subjects.java
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ public static String getDn(Subject subject)
* primary FQANs
*/
public static FQAN getPrimaryFqan(Subject subject)
throws NoSuchElementException
throws IllegalArgumentException
{
Set<FQANPrincipal> principals =
subject.getPrincipals(FQANPrincipal.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,7 @@ void toHtmlRow(List<String> out) {
if (moverStatus != null) {
out.add(moverStatus);
out.add(elapsedSinceSubmitted);
if (moverStart != null) {
out.add(transferTimeStr);
out.add(String.valueOf(bytesTransferred));
out.add(transferRateStr);
out.add(running);
}
addMoverInfo(out);
}
}

Expand All @@ -240,13 +235,25 @@ void toAsciiLine(StringBuilder builder) {
args.add(replyHost);
args.add(sessionStatus);
args.add(waiting);
args.add(moverStatus == null ? "No-mover()-Found" : moverStatus);
args.add(transferTimeStr);
args.add(String.valueOf(bytesTransferred));
args.add(transferRateStr);
args.add(running);

if (moverStatus == null) {
args.add("No-mover()-Found");
} else {
args.add(moverStatus);
addMoverInfo(args);
}

builder.append(new Args(args)).append('\n');
}

private void addMoverInfo(List<String> list) {
if (moverStart != null) {
list.add(transferTimeStr);
list.add(String.valueOf(bytesTransferred));
list.add(transferRateStr);
list.add(running);
}
}
}

public TransferObserverV1(String name, String args) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,14 @@ public Subject getSubject() {
}

public Long getTransferRate() {
if (bytesTransferred == null) {
return 0L;
}

if (transferTime == null) {
return 0L;
}

long kB = bytesTransferred / 1024;
long secs = transferTime / 1000;
return secs > 0.0 ? kB / secs : 0L;
Expand Down
48 changes: 45 additions & 3 deletions modules/dcache/src/main/java/diskCacheV111/util/UserInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,12 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
*/
package diskCacheV111.util;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.security.auth.Subject;
import java.io.Serializable;
import java.util.NoSuchElementException;

import org.dcache.auth.FQAN;
import org.dcache.auth.Subjects;
Expand All @@ -70,6 +74,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
* between core dCache and webadmin modules.</p>
*/
public final class UserInfo implements Serializable {
private static final Logger LOGGER = LoggerFactory.getLogger(UserInfo.class);
private String username;
private Long uid;
private Long gid;
Expand All @@ -80,9 +85,46 @@ public UserInfo() {

public UserInfo(Subject subject) {
username = Subjects.getUserName(subject);
uid = Subjects.getUid(subject);
gid = Subjects.getPrimaryGid(subject);
primaryFqan = Subjects.getPrimaryFqan(subject);

/*
* The values may not be set by some protocols,
* such as anonymous dcap. We do not want to fail here
* in that case.
*
* In the case of an IllegalArgument, this is actually an
* error which should be reported, but again we do not
* want to fail the interface because of it.
*/

if (Subjects.isNobody(subject)) {
uid = null;
} else {
try {
uid = Subjects.getUid(subject);
} catch (IllegalArgumentException e) {
uid = null;
LOGGER.warn("Error when fetching UID from {}: {}.",
subject, e.toString());
}
}

try {
gid = Subjects.getPrimaryGid(subject);
} catch (NoSuchElementException e) {
gid = null;
} catch (IllegalArgumentException e) {
gid = null;
LOGGER.warn("Error when fetching GID from {}: {}.",
subject, e.toString());
}

try {
primaryFqan = Subjects.getPrimaryFqan(subject);
} catch (IllegalArgumentException e) {
primaryFqan = null;
LOGGER.warn("Error when fetching primary FQAN from {}: {}.",
subject, e.toString());
}
}

public String getGid() {
Expand Down

0 comments on commit c39133d

Please sign in to comment.