InsightSoftwareConsortium / InsightSoftwareConsortium/ITK

ShapedNeighborhoodIterator silently breaks const-correctness on image argument

Open
#6,102 1 comment 0 reactions 1 assignee Claimed by @N-Dekker View on GitHub
type:Bug
Dominant language
C++
Stars
1.7k
Forks
748
Avg merge
1d 1h
Merged PRs (30d)
64

Description

### Description

ShapedNeighborhoodIterator has a constructor that accepts a pointer to a const `Image`:

```c++
ShapedNeighborhoodIterator(const SizeType & radius, const ImageType * ptr, const RegionType & region)
```

At https://github.com/InsightSoftwareConsortium/ITK/blob/056f5eb93831a2dc3a6afe68a1aa33e8b20bb8ea/Modules/Core/Common/include/itkShapedNeighborhoodIterator.h#L222

This offers a "backdoor" for users to modify an image via a pointer-to-const, thereby breaking logical const-correctness. It is also unusual within ITK. Constructors of other non-const ITK iterators do not have such a `const ImageType *` parameter.

### Steps to Reproduce

The following code shows how to modify an image via a `const ImageType`, by exploiting this ShapedNeighborhoodIterator bug. It does so by calling `shapedNeighborhoodIterator.SetCenterPixel(1)`, but it might as well do so iteratively, by setting the pixel values from `shapedNeighborhoodIterator.Begin()` to `shapedNeighborhoodIterator.End()`

```c++
#include "itkShapedNeighborhoodIterator.h"
#include "itkImageBufferRange.h"

void
TestModifyingConstImage()
{
static constexpr unsigned int dimension{ 2 };
using ImageType = itk::Image;

constexpr auto modifyConstImage = [](const ImageType * const constImage) {
constexpr auto radius = itk::Size::Filled(1);
const auto region = constImage->GetBufferedRegion();

itk::ShapedNeighborhoodIterator shapedNeighborhoodIterator(radius, constImage, region);

shapedNeighborhoodIterator.SetLocation(itk::Index::Filled(1));
shapedNeighborhoodIterator.SetCenterPixel(1);
};

const auto image = ImageType::New();
image->SetRegions(itk::Size::Filled(3));
image->AllocateInitialized();

const auto printImage = [image] {
for (const int pixelValue : itk::ImageBufferRange(*image))
{
std::cout << pixelValue;
}
std::cout << '\n';
};

printImage(); // Prints "000000000".
modifyConstImage(image);
printImage(); // Prints "000010000"!
}

```
### Expected behavior

The following line should not compile. `ShapedNeighborhoodIterator` should not accept a "const" image as argument.

```c++
itk::ShapedNeighborhoodIterator shapedNeighborhoodIterator(radius, constImage, region);
```

### Actual behavior

It prints the following lines, indicating the center pixel value of the 3x3 test image has changed.
```
000000000
000010000
```

### Reproducibility

100%

### Versions

Today's main revision: commit 056f5eb93831a2dc3a6afe68a1aa33e8b20bb8ea

### Environment

Any supported ITK platform

### Additional Information

I'm considering to submit a pull request to fix this issue.

----

For the record, it appears that this mistake (having a `const ImageType *` constructor parameter) was there already when ShapedNeighborhoodIterator was introduced, with commit fc6d1b23d237136144c009c075941fdaaf7d2cd3, Jan 8, 2003: https://github.com/InsightSoftwareConsortium/ITK/blob/fc6d1b23d237136144c009c075941fdaaf7d2cd3/Code/Common/itkShapedNeighborhoodIterator.h#L181-L183

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.