[PR #119] [MERGED] Check availability before conditionals #167

New issue

Closed

opened 2026-02-27 08:12:10 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 08:12:10 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kokarare1212/librespot-python/pull/119
Author: @yeralin
Created: 3/10/2022
Status: ✅ Merged
Merged: 3/10/2022
Merged by: @kokarare1212

Base: rewrite ← Head: fix-read-stream

📝 Commits (1)

90c35b4 Check availability before conditionals

📊 Changes

1 file changed (+1 additions, -2 deletions)

View changed files

📝 librespot/audio/__init__.py (+1 -2)

📄 Description

While using librespot-python library I encountered a bug while reading from a stream.

buffer = io.BytesIO()
chunk = int(self.__pos / (128 * 1024))
chunk_off = int(self.__pos % (128 * 1024))
chunk_end = int(__size / (128 * 1024))
chunk_end_off = int(__size % (128 * 1024))
...
if chunk_off + __size > len(self.buffer()[chunk]): # culprit
    self.check_availability(chunk, True, False)
    buffer.write(self.buffer()[chunk][chunk_off:])
    chunk += 1
    while chunk <= chunk_end:
        self.check_availability(chunk, True, False)
        if chunk == chunk_end:
            buffer.write(self.buffer()[chunk][:chunk_end_off])
        else:
            buffer.write(self.buffer()[chunk])
        chunk += 1
else:
    self.check_availability(chunk, True, False)
    buffer.write(self.buffer()[chunk][chunk_off:chunk_off + __size])

When we are loading the last bytes of the current chunk byte-array self.buffer()[chunk], on the next iteration we will try to access self.buffer()[chunk+1] which is not loaded yet i.e. len(self.buffer()[chunk+1]) == 0.

This causes if chunk_off + __size > len(self.buffer()[chunk]) statement to always resolve to True which then writes an entire self.buffer()[chunk+1] into the buffer as opposed to writing only requested part of it of __size.

Therefore, I moved self.check_availability(chunk, True, False) to the top, so that len(self.buffer()[chunk+1]) gets loaded before these conditional checks get executed.

Then, len(self.buffer()[chunk+1]) > 0 and the conditional logic works as expected.

Let me know if you need any further clarifications.

I don't foresee any risks with this PR, since we call self.check_availability(chunk, True, False) after these checks in any case.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kokarare1212/librespot-python/pull/119 **Author:** [@yeralin](https://github.com/yeralin) **Created:** 3/10/2022 **Status:** ✅ Merged **Merged:** 3/10/2022 **Merged by:** [@kokarare1212](https://github.com/kokarare1212) **Base:** `rewrite` ← **Head:** `fix-read-stream` --- ### 📝 Commits (1) - [`90c35b4`](https://github.com/kokarare1212/librespot-python/commit/90c35b450a614af037f81842804d6938781c7dab) Check availability before conditionals ### 📊 Changes **1 file changed** (+1 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `librespot/audio/__init__.py` (+1 -2) </details> ### 📄 Description While using `librespot-python` library I encountered a bug while reading from a stream. ```python buffer = io.BytesIO() chunk = int(self.__pos / (128 * 1024)) chunk_off = int(self.__pos % (128 * 1024)) chunk_end = int(__size / (128 * 1024)) chunk_end_off = int(__size % (128 * 1024)) ... if chunk_off + __size > len(self.buffer()[chunk]): # culprit self.check_availability(chunk, True, False) buffer.write(self.buffer()[chunk][chunk_off:]) chunk += 1 while chunk <= chunk_end: self.check_availability(chunk, True, False) if chunk == chunk_end: buffer.write(self.buffer()[chunk][:chunk_end_off]) else: buffer.write(self.buffer()[chunk]) chunk += 1 else: self.check_availability(chunk, True, False) buffer.write(self.buffer()[chunk][chunk_off:chunk_off + __size]) ``` When we are loading the last bytes of the current chunk byte-array `self.buffer()[chunk]`, on the next iteration we will try to access `self.buffer()[chunk+1]` which is not loaded yet i.e. `len(self.buffer()[chunk+1]) == 0`. ![image](https://user-images.githubusercontent.com/8620461/157594150-b995fccb-14c6-4076-9110-78b8e02e95dc.png) This causes `if chunk_off + __size > len(self.buffer()[chunk])` statement **to always resolve to `True`** which then **writes an entire `self.buffer()[chunk+1]` into the buffer** as opposed to writing only requested part of it of `__size`. Therefore, I moved `self.check_availability(chunk, True, False)` to the top, so that `len(self.buffer()[chunk+1])` gets loaded before these conditional checks get executed. Then, `len(self.buffer()[chunk+1]) > 0` and the conditional logic works as expected. Let me know if you need any further clarifications. I don't foresee any risks with this PR, since we call `self.check_availability(chunk, True, False)` after these checks in any case. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>