Skip to content

Commit 453e00d

Browse files
fix: redirect to book page with flash message after returning a book
1 parent 71dd632 commit 453e00d

2 files changed

Lines changed: 34 additions & 5 deletions

File tree

openlibrary/i18n/messages.pot

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,6 +1427,15 @@ msgstr ""
14271427
msgid "Loan History"
14281428
msgstr ""
14291429

1430+
#: openlibrary/plugins/upstream/borrow.py
1431+
msgid "this book"
1432+
msgstr ""
1433+
1434+
#: openlibrary/plugins/upstream/borrow.py
1435+
#, python-format
1436+
msgid "You have successfully returned %s."
1437+
msgstr ""
1438+
14301439
#: openlibrary/plugins/upstream/borrow.py
14311440
msgid ""
14321441
"Your account has hit a lending limit. Please try again later or contact "

openlibrary/plugins/upstream/borrow.py

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Handlers for borrowing books"""
22

3+
import contextlib
34
import copy
45
import hashlib
56
import hmac
@@ -161,7 +162,20 @@ def POST(self, key): # noqa: PLR0915
161162
raise web.seeother(archive_url)
162163

163164
error_redirect = archive_url
164-
edition_redirect = urllib.parse.quote(i.redirect or edition.url())
165+
166+
# Sanitize i.redirect to prevent open redirect attacks by stripping
167+
# any host/scheme from the URL, preserving only the path and query string.
168+
parsed_redirect = urllib.parse.urlsplit(i.redirect) if i.redirect else None
169+
redirect_path = (
170+
(
171+
parsed_redirect.path
172+
+ ('?' + parsed_redirect.query if parsed_redirect.query else '')
173+
)
174+
if parsed_redirect
175+
else ''
176+
)
177+
edition_redirect = redirect_path or edition.url()
178+
165179
user = accounts.get_current_user()
166180

167181
if user:
@@ -173,18 +187,24 @@ def POST(self, key): # noqa: PLR0915
173187
) # invalidate cache for user loans
174188
if not user or not ia_itemname or not s3_keys:
175189
web.setcookie(config.login_cookie_name, "", expires=-1)
176-
redirect_url = (
177-
f"/account/login?redirect={edition_redirect}/borrow?action={action}"
178-
)
190+
redirect_url = f"/account/login?redirect={urllib.parse.quote(edition_redirect, safe='')}%2Fborrow%3Faction%3D{action}"
179191
if i._autoReadAloud is not None:
180192
redirect_url += '&_autoReadAloud=' + i._autoReadAloud
181193
raise web.seeother(redirect_url)
182194

183195
if action == 'return':
184-
lending.s3_loan_api(s3_keys, ocaid=edition.ocaid, action='return_loan')
196+
# Suppress PatronAccessException: the loan may have already expired
197+
# on the IA side. Either way, proceed with redirect and confirmation.
198+
with contextlib.suppress(lending.PatronAccessException):
199+
lending.s3_loan_api(s3_keys, ocaid=edition.ocaid, action='return_loan')
185200
stats.increment('ol.loans.return')
186201
edition.update_loan_status()
187202
user.update_loan_status()
203+
title = edition.title or _('this book')
204+
add_flash_message(
205+
'success',
206+
_('You have successfully returned %s.') % title,
207+
)
188208
raise web.seeother(edition_redirect)
189209
elif action == 'join-waitinglist':
190210
lending.get_cached_user_waiting_loans.memcache_delete(

0 commit comments

Comments
 (0)