Skip to:
Content

Opened 22 months ago

Last modified 12 months ago

#3125 new enhancement

`bbp_get_topic_id()` might incorrectly return a reply_id or forum_id

Reported by: casiepa Owned by: johnjamesjacoby
Milestone: 2.8 Priority: normal
Severity: normal Version:
Component: General Keywords: needs-patch needs-unit-tests
Cc: contato@…

Description (last modified by netweb)

Calling the bbp_get_topic_id() function will check if $id is a valid number by using ( ! empty( $topic_id ) && is_numeric( $topic_id ) ).
If function bbp_get_topic_id() is called with a forum_id or a reply_id, it will still accept the value.

This behaviour should be changed, but there might be 2 ways:
1) if $id is a topic, return $id, otherwise 0 (so add the logic of bbp_is_topic to make sure $id is indeed a topic)
or
2) if $id is an ID for a reply, return the corresponding ID for the topic this reply is part of (ID crawling)

Regardless of choosing 1) or 2) apply the same logic to bbp_get_forum_id() and bbp_get_reply_id().

Change History (3)

#1 @espellcaste
22 months ago

  • Cc contato@… added

I'm in favor of 1 and not 2. I see bbp_get_topic_id as a topic id catcher, not a catch, decides what it is and then returns what it comes with it.

bbp_is_topic was not used in the first place because of performance concerns, so unless a performant crawler is introduced, let's try to minimize the hit on a highly used function.

What do you think?

Last edited 22 months ago by espellcaste (previous) (diff)

#2 @netweb
22 months ago

  • Description modified (diff)
  • Keywords needs-patch needs-unit-tests added
  • Summary changed from bbp_get_topic_id might incorrectly give back a reply_id or forum_id to `bbp_get_topic_id()` might incorrectly return a reply_id or forum_id

#3 @johnjamesjacoby
12 months ago

  • Milestone changed from Awaiting Review to 2.8
  • Owner set to johnjamesjacoby

It's true that some of these functions are just pass-through's.

They don't verify that the ID being passed in actually belongs to the exact type of thing.

We can be more strict about this in a bunch of places, if we want.

Moving to 2.8 to see if we need to.

Note: See TracTickets for help on using tickets.