Skip to content

WW-5646 Modernize path normalization in Include component#1780

Open
arunmanni-ai wants to merge 3 commits into
apache:mainfrom
arunmanni-ai:modernize-include-path-normalization
Open

WW-5646 Modernize path normalization in Include component#1780
arunmanni-ai wants to merge 3 commits into
apache:mainfrom
arunmanni-ai:modernize-include-path-normalization

Conversation

@arunmanni-ai

Copy link
Copy Markdown
Contributor

The path normalization logic in Include.getContextRelativePath() used the
legacy java.util.Stack class and did not handle the edge case where
dot-dot segments exceed the available path depth.

This patch:

  • Replaces java.util.Stack with ArrayDeque (recommended since JDK 6)
  • Adds an isEmpty() guard before pop() to handle excess dot-dot gracefully
  • Uses enhanced for-loop instead of indexed elementAt() iteration
  • Adds test coverage for edge case dot-dot paths

@arunmanni-ai arunmanni-ai changed the title Modernize path normalization in Include component WW-5646 Modernize path normalization in Include component Jul 11, 2026
The for-each loop iterated the ArrayDeque head-to-tail (most
recently pushed first), which is the reverse of the old Stack's
insertion-order iteration. This caused rebuilt paths like
"car/view.jsp" to come out as "view.jsp/car".

Use descendingIterator() to restore the original oldest-first
ordering when rebuilding the flat path string.
@arunmanni-ai

Copy link
Copy Markdown
Contributor Author

Fixed a regression introduced by the Stack → ArrayDeque swap: the rebuild loop iterated the deque head-to-tail (most recently pushed first), which is the reverse of Stack's insertion-order iteration. This caused rebuilt paths like car/view.jsp to come out as view.jsp/car, which is what the CI failures were catching.

Fixed by using descendingIterator() to restore the original oldest-first ordering when rebuilding the flat path string. The traversal-prevention logic (tokenizing, .. → pop, the isEmpty() guard clamping excess .. at root) is unchanged.

Verified locally — all passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant