Commit ea6f71a3 authored by Tigran Mkrtchyan's avatar Tigran Mkrtchyan
Browse files

nfs3: fix readdir entry size calculation

Motivation:
when estimating readdir reply size we should not forget about boolean
flag that indicates the presence of a next entry.

Modification:
fix entry overhead size.

Result:
The returned xdr doesn't overflow expected reply size.

Acked-by: Paul Millar
Target: master, 0.22
parent 9c320994
Pipeline #1876 passed with stage
in 1 minute and 20 seconds
/*
* Copyright (c) 2009 - 2020 Deutsches Elektronen-Synchroton,
* Copyright (c) 2009 - 2021 Deutsches Elektronen-Synchroton,
* Member of the Helmholtz Association, (DESY), HAMBURG, GERMANY
*
* This library is free software; you can redistribute it and/or modify
......@@ -155,7 +155,18 @@ import javax.security.auth.Subject;
public class NfsServerV3 extends nfs3_protServerStub {
// needed to calculate replay size for READDIR3 and READDIRPLUS3
private static final int ENTRY3_SIZE = 24;
/*
* readdir entry size + overhead
* 8 (cookie) + 8 (file id) + 4 (name len) + 4 (smallest padded name) + 4 (boolean has next)
*/
private static final int ENTRY3_SIZE = 28;
/*
* readdir_plus entry size + overhead, without file handle
* 8 (cookie) + 8 (file id) + 4 (name len) + 4 (smallest padded name) + 4 (boolean has next) +
* 4 (boolean attrs follow, always true) + 84 (fattr3) + 4 (boolean has handle, always true) +
* 4 (handle len)
*/
private static final int ENTRYPLUS3_SIZE = 124;
private static final int READDIR3RESOK_SIZE = 104;
private static final int READDIRPLUS3RESOK_SIZE = 104;
......
......@@ -24,6 +24,10 @@ import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import static org.dcache.testutils.XdrHelper.calculateSize;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.mockito.Mockito.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
......@@ -177,7 +181,8 @@ public class NfsServerV3READDIRPLUS_3Test {
n++;
}
assertEquals("Not all antries returned", dirContents.size() - 1, n);
assertThat("reply overflow", calculateSize(result), is(lessThanOrEqualTo(3480)));
assertEquals("Not all entries returned", dirContents.size() - 1, n);
assertFalse("The last entry is missed", result.resok.reply.eof);
}
}
......@@ -24,6 +24,10 @@ import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import static org.dcache.testutils.XdrHelper.calculateSize;
import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.*;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.mockito.Mockito.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
......@@ -165,7 +169,7 @@ public class NfsServerV3READDIR_3Test {
when(vfs.list(eq(dirInode), any(), anyLong())).thenReturn(new DirectoryStream(cookieVerifier, dirContents));
RpcCall call = new RpcCallBuilder().from("1.2.3.4", "someHost.acme.com", 42).nfs3().noAuth().build();
READDIR3args args = NfsV3Ops.readDir(dirHandle, 0, cookieVerifier, 770); // only 22 entries will fit
READDIR3args args = NfsV3Ops.readDir(dirHandle, 0, cookieVerifier, 836); // only 22 entries will fit
READDIR3res result = nfsServer.NFSPROC3_READDIR_3(call, args);
int n = 0;
......@@ -176,7 +180,8 @@ public class NfsServerV3READDIR_3Test {
n++;
}
assertEquals("Not all antries returned", dirContents.size() - 1, n);
assertThat("reply overflow", calculateSize(result), is(lessThanOrEqualTo(836)));
assertEquals("Not all entries returned", dirContents.size() - 1, n);
assertFalse("The last entry is missed", result.resok.reply.eof);
}
}
......@@ -27,8 +27,16 @@ import org.dcache.oncrpc4j.xdr.Xdr;
import org.junit.Test;
import org.junit.Before;
import static org.hamcrest.MatcherAssert.*;
import static org.dcache.nfs.v4.NfsTestUtils.generateRpcCall;
import static org.junit.Assert.*;
import static org.dcache.testutils.XdrHelper.calculateSize;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.*;
public class OperationREADDIRTest {
......@@ -111,6 +119,7 @@ public class OperationREADDIRTest {
listed();
assertThat("reply overflow", calculateSize(result), is(lessThanOrEqualTo(1000)));
assertEquals("Not all entries returned", dirEntries.length - 1, entryCount());
assertFalse("The last entry is missed", result.opreaddir.resok4.reply.eof);
}
......
package org.dcache.testutils;
import org.dcache.oncrpc4j.xdr.Xdr;
import org.dcache.oncrpc4j.xdr.XdrAble;
public class XdrHelper {
private XdrHelper() {}
public static int calculateSize (XdrAble xdrAble) {
try {
Xdr xdr = new Xdr(128);
xdr.beginEncoding();
xdrAble.xdrEncode(xdr);
xdr.endEncoding();
return xdr.getBytes().length;
} catch (Exception e) {
throw new AssertionError("object does not survive xdr encoding", e);
}
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment