Skip to content

Commit 80ededd

Browse files
sonivijaykVijay Sonijamshale
authored
fix: encode revocation tag in tails upload URL (issue 1580) (#3996)
* fix: encode revocation tag in tails upload URL (issue 1580) Signed-off-by: Vijay Soni <vijaysoni@sonivijay.com> * fixed linting issue Signed-off-by: Vijay Soni <vijaysoni@sonivijay.com> * fixed sonar quality gate issues Signed-off-by: Vijay Soni <vijaysoni@sonivijay.com> * fixed test data and comment issues reported by copilot and sonar Signed-off-by: Vijay Soni <vijaysoni@sonivijay.com> --------- Signed-off-by: Vijay Soni <vijaysoni@sonivijay.com> Co-authored-by: Vijay Soni <vijaysoni@sonivijay.com> Co-authored-by: jamshale <31809382+jamshale@users.noreply.github.com>
1 parent 4cd6a55 commit 80ededd

2 files changed

Lines changed: 54 additions & 22 deletions

File tree

‎acapy_agent/tails/indy_tails_server.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import logging
44
from typing import Tuple
5+
from urllib.parse import quote
56

67
from ..config.injection_context import InjectionContext
78
from ..ledger.base import BaseLedger
@@ -70,8 +71,10 @@ async def upload_tails_file(
7071
"tails_server_base_url setting is not set"
7172
)
7273

73-
upload_url = tails_server_upload_url.rstrip("/") + f"/{filename}"
74-
public_url = tails_server_base_url.rstrip("/") + f"/{filename}"
74+
# BUG #1580: encode revocation tag to avoid spaces in tails URLs
75+
encoded_filename = quote(filename, safe=":")
76+
upload_url = tails_server_upload_url.rstrip("/") + f"/{encoded_filename}"
77+
public_url = tails_server_base_url.rstrip("/") + f"/{encoded_filename}"
7578

7679
try:
7780
await put_file(

‎acapy_agent/tails/tests/test_indy.py‎

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from unittest import IsolatedAsyncioTestCase
2+
from urllib.parse import quote
23

34
from ...config.injection_context import InjectionContext
45
from ...ledger.base import BaseLedger
@@ -10,6 +11,7 @@
1011
TEST_DID = "55GkHamhTU1ZbTbV2ab9DE"
1112
CRED_DEF_ID = f"{TEST_DID}:3:CL:1234:default"
1213
REV_REG_ID = f"{TEST_DID}:4:{CRED_DEF_ID}:CL_ACCUM:0"
14+
REV_REG_ID_WITH_SPACE = f"{TEST_DID}:4:{CRED_DEF_ID}:CL_ACCUM:tag with space"
1315

1416

1517
class TestIndyTailsServer(IsolatedAsyncioTestCase):
@@ -18,14 +20,14 @@ async def test_upload_no_tails_upload_url_x(self):
1820
indy_tails = test_module.IndyTailsServer()
1921

2022
with self.assertRaises(test_module.TailsServerNotConfiguredError):
21-
await indy_tails.upload_tails_file(context, REV_REG_ID, "/tmp/dummy/path")
23+
await indy_tails.upload_tails_file(context, REV_REG_ID, "dummy/path")
2224

2325
async def test_upload(self):
2426
context = InjectionContext(
2527
settings={
2628
"ledger.genesis_transactions": "dummy",
27-
"tails_server_base_url": "http://1.2.3.4:8088/tails/",
28-
"tails_server_upload_url": "http://1.2.3.4:8088",
29+
"tails_server_base_url": "https://tails.example/tails/",
30+
"tails_server_upload_url": "https://tails.example",
2931
}
3032
)
3133
indy_tails = test_module.IndyTailsServer()
@@ -35,21 +37,21 @@ async def test_upload(self):
3537
(ok, text) = await indy_tails.upload_tails_file(
3638
context,
3739
REV_REG_ID,
38-
"/tmp/dummy/path",
40+
"dummy/path",
3941
)
4042
assert ok
4143

42-
# already contains / from config, no need to add it
43-
assert text == context.settings["tails_server_base_url"] + REV_REG_ID
44-
assert (
45-
mock_put.call_args.args[0]
46-
== context.settings["tails_server_upload_url"] + "/" + REV_REG_ID
44+
assert text == context.settings["tails_server_base_url"] + quote(
45+
REV_REG_ID, safe=":"
4746
)
47+
assert mock_put.call_args.args[0] == context.settings[
48+
"tails_server_upload_url"
49+
] + "/" + quote(REV_REG_ID, safe=":")
4850

4951
async def test_upload_indy_vdr(self):
5052
self.profile = await create_test_profile()
51-
self.profile.settings["tails_server_base_url"] = "http://1.2.3.4:8088/tails/"
52-
self.profile.settings["tails_server_upload_url"] = "http://1.2.3.4:8088"
53+
self.profile.settings["tails_server_base_url"] = "https://tails.example/tails/"
54+
self.profile.settings["tails_server_upload_url"] = "https://tails.example"
5355
mock_multi_ledger_manager = mock.MagicMock(
5456
BaseMultipleLedgerManager, autospec=True
5557
)
@@ -74,23 +76,50 @@ async def test_upload_indy_vdr(self):
7476
(ok, text) = await indy_tails.upload_tails_file(
7577
self.profile.context,
7678
REV_REG_ID,
77-
"/tmp/dummy/path",
79+
"dummy/path",
7880
)
7981
assert ok
8082

81-
# already contains / from config, no need to add it
82-
assert text == self.profile.settings["tails_server_base_url"] + REV_REG_ID
83-
assert (
84-
mock_put.call_args.args[0]
85-
== self.profile.settings["tails_server_upload_url"] + "/" + REV_REG_ID
83+
assert text == self.profile.settings["tails_server_base_url"] + quote(
84+
REV_REG_ID, safe=":"
8685
)
86+
assert mock_put.call_args.args[0] == self.profile.settings[
87+
"tails_server_upload_url"
88+
] + "/" + quote(REV_REG_ID, safe=":")
89+
90+
async def test_upload_with_space_in_revocation_tag(self):
91+
# BUG #1580: ensure revocation tag whitespace is URL-encoded
92+
context = InjectionContext(
93+
settings={
94+
"ledger.genesis_transactions": "dummy",
95+
"tails_server_base_url": "https://tails.example/tails/",
96+
"tails_server_upload_url": "https://tails.example",
97+
}
98+
)
99+
indy_tails = test_module.IndyTailsServer()
100+
101+
with mock.patch.object(test_module, "put_file", mock.CoroutineMock()) as mock_put:
102+
mock_put.return_value = "tails-hash"
103+
(ok, text) = await indy_tails.upload_tails_file(
104+
context,
105+
REV_REG_ID_WITH_SPACE,
106+
"dummy/path",
107+
)
108+
assert ok
109+
110+
assert text == context.settings["tails_server_base_url"] + quote(
111+
REV_REG_ID_WITH_SPACE, safe=":"
112+
)
113+
assert mock_put.call_args.args[0] == context.settings[
114+
"tails_server_upload_url"
115+
] + "/" + quote(REV_REG_ID_WITH_SPACE, safe=":")
87116

88117
async def test_upload_x(self):
89118
context = InjectionContext(
90119
settings={
91120
"ledger.genesis_transactions": "dummy",
92-
"tails_server_base_url": "http://1.2.3.4:8088/tails/",
93-
"tails_server_upload_url": "http://1.2.3.4:8088",
121+
"tails_server_base_url": "https://tails.example/tails/",
122+
"tails_server_upload_url": "https://tails.example",
94123
}
95124
)
96125
indy_tails = test_module.IndyTailsServer()
@@ -99,7 +128,7 @@ async def test_upload_x(self):
99128
mock_put.side_effect = test_module.PutError("Server down for maintenance")
100129

101130
(ok, text) = await indy_tails.upload_tails_file(
102-
context, REV_REG_ID, "/tmp/dummy/path"
131+
context, REV_REG_ID, "dummy/path"
103132
)
104133
assert not ok
105134
assert text == "Server down for maintenance"

0 commit comments

Comments
 (0)