Repository: scionproto/scion
Pull request #4242 (2022-08-03 13:11:10)

Return an error when reversing an empty SCION path
Fixes issue #4241.

I introduced a separate test `TestEmptyRawReverse` instead of adding an extra case to `TestRawReverse` because the test cases of the latter are shared with the test for reversing `Decoded` paths, which expects that reversing a `Decoded` yields a `Decoded`. Putting everything in the same test can be done with a bigger change-set though.

<!-- Reviewable:start -->
- - -
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/scionproto/scion/4242)
<!-- Reviewable:end -->



Patch:
diff --git a/pkg/slayers/path/scion/decoded.go b/pkg/slayers/path/scion/decoded.go
index 231ca140d..da183f290 100644
--- a/pkg/slayers/path/scion/decoded.go
+++ b/pkg/slayers/path/scion/decoded.go
@@ -93,8 +93,7 @@ func (s *Decoded) SerializeTo(b []byte) error {
 // Reverse reverses a SCION path.
 func (s *Decoded) Reverse() (path.Path, error) {
 	if s.NumINF == 0 {
-		// Empty path doesn't need reversal.
-		return nil, nil
+		return nil, serrors.New("empty decoded path is invalid and cannot be reversed")
 	}
 	// Reverse order of InfoFields and SegLens
 	for i, j := 0, s.NumINF-1; i < j; i, j = i+1, j-1 {


Test patch:
diff --git a/pkg/slayers/path/scion/decoded_test.go b/pkg/slayers/path/scion/decoded_test.go
index fc215d8a2..096f26972 100644
--- a/pkg/slayers/path/scion/decoded_test.go
+++ b/pkg/slayers/path/scion/decoded_test.go
@@ -81,6 +81,12 @@ var decodedTestPath = &scion.Decoded{
 	HopFields:  testHopFields,
 }
 
+var emptyDecodedTestPath = &scion.Decoded{
+	Base:       scion.Base{},
+	InfoFields: []path.InfoField{},
+	HopFields:  []path.HopField{},
+}
+
 var rawPath = []byte("\x00\x00\x20\x80\x00\x00\x01\x11\x00\x00\x01\x00\x01\x00\x02\x22\x00\x00" +
 	"\x01\x00\x00\x3f\x00\x01\x00\x00\x01\x02\x03\x04\x05\x06\x00\x3f\x00\x03\x00\x02\x01\x02\x03" +
 	"\x04\x05\x06\x00\x3f\x00\x00\x00\x02\x01\x02\x03\x04\x05\x06\x00\x3f\x00\x01\x00\x00\x01\x02" +
@@ -180,6 +186,11 @@ func TestDecodedReverse(t *testing.T) {
 	}
 }
 
+func TestEmptyDecodedReverse(t *testing.T) {
+	_, err := emptyDecodedTestPath.Reverse()
+	assert.Error(t, err)
+}
+
 func TestDecodedToRaw(t *testing.T) {
 	raw, err := decodedTestPath.ToRaw()
 	assert.NoError(t, err)
diff --git a/pkg/slayers/path/scion/raw_test.go b/pkg/slayers/path/scion/raw_test.go
index db23bfda8..ff527d939 100644
--- a/pkg/slayers/path/scion/raw_test.go
+++ b/pkg/slayers/path/scion/raw_test.go
@@ -38,6 +38,19 @@ var rawTestPath = &scion.Raw{
 	Raw: rawPath,
 }
 
+var emptyRawTestPath = &scion.Raw{
+	Base: scion.Base{
+		PathMeta: scion.MetaHdr{
+			CurrINF: 0,
+			CurrHF:  0,
+			SegLen:  [3]uint8{0, 0, 0},
+		},
+		NumINF:  0,
+		NumHops: 0,
+	},
+	Raw: make([]byte, scion.MetaLen),
+}
+
 func TestRawSerialize(t *testing.T) {
 	b := make([]byte, rawTestPath.Len())
 	assert.NoError(t, rawTestPath.SerializeTo(b))
@@ -75,6 +88,11 @@ func TestRawReverse(t *testing.T) {
 	}
 }
 
+func TestEmptyRawReverse(t *testing.T) {
+	_, err := emptyRawTestPath.Reverse()
+	assert.Error(t, err)
+}
+
 func TestRawToDecoded(t *testing.T) {
 	decoded, err := rawTestPath.ToDecoded()
 	assert.NoError(t, err)